[5.7] RequirementMachine: Disable redundant requirement diagnostics by default #58814
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DougGregor
approved these changes
May 11, 2022
…dundant by an inferred requirement
We infer requirements from types appearing in parameter and result types,
like this:
func foo<T>(_: Set<T>) // 'T : Hashable' inferred from 'Set<T>'
Normally we muffle the warning if the requirement is re-stated redundantly:
func foo<T>(_: Set<T>) where T : Hashable // no warning
However, in some cases we failed to do this if the requirement was inferred
from a type appearing in a 'where' clause, like this:
struct G<A, B> {}
extension G where B : Hashable, A == Set<B> {}
This is because in this case the redundancy was detected by
RewriteSystem::addRule() returning false.
The simplest fix here is to change InferredGenericSignatureRequest
to re-order requirements so that inferred requirements appear last.
This way, if any are redundant, we won't diagnose them since it is
the inferred requirement that is redundant and not the user-written
one.
Fixes rdar://problem/92092635.
…agnostics A handful of cases where we emit a bogus redundancy warning are marked with 'FIXME(rqm-diagnostics)'.
…t and add -warn-redundant-requirements frontend flag
slavapestov
force-pushed
the
rqm-tweak-diagnostics-5.7
branch
from
98b14dd to
867adbd
Compare
|
@swift-ci Please test |
hborla
approved these changes
May 12, 2022
AnthonyLatsis
added
🍒 release cherry pick
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #58780.
This PR fixes an issue where we failed to muffle redundancy diagnostics involving inferred requirements (rdar://problem/92092635, rdar://problem/92122203).
But also, it disables redundant requirement diagnostics unless the user passes in the -warn-redundant-requirements frontend flag. I updated existing tests that expect to see these diagnostics to pass in this flag explicitly.
Our reasoning here is that these diagnostics are rarely useful, and furthermore, they do not fit the existing philosophy behind warnings. They do not actually indicate a potential problem with the code. We don't diagnose redundant type annotations elsewhere, for instance. Sometimes, users actually want to re-state redundant requirements for readability, because the derivation path for a redundant requirement might be far from obvious, if it involves multiple same-type requirements for example.