Skip to content

RequirementMachine: Disable redundant requirement diagnostics by default #58780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 10, 2022

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented May 10, 2022

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.

…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 slavapestov force-pushed the rqm-tweak-diagnostics branch from e1c9be0 to f39372b Compare May 10, 2022 05:50
@slavapestov slavapestov requested a review from hborla May 10, 2022 05:50
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant