Warn about unnecessary availability independently from the deployment target #37350
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
|
@swift-ci please test |
xymus
approved these changes
May 12, 2021
| @@ -0,0 +1,12 @@ | |||
| // Ensure that the `unnecessary check` availability warning is emitted when unnecessary due to | |||
| // scope's explicit annotation | |||
There was a problem hiding this comment.
Should this be the description for the other test?
There was a problem hiding this comment.
Yes, thanks for catching this!
…yment target This change separates emission of the diagnostics like: ``` unnecessary check for 'Platform'; enclosing scope ensures guard will always be true ``` from the deployment target of the current compilation. Instead, these diagnostics will only be emitted if the enclosing scope guard is explicitly specified by the user with an `#availability` attribute. This fixes cases like the following: ``` @available(macOS 11.0, *) class Foo { func foo() { if #available(macOS 11.1, *) {} } } ``` Compiling this with `-target x86_64-apple-macos11.2` results in: ``` warning: unnecessary check for 'macOS'; enclosing scope ensures guard will always be true if #available(macOS 11.1, *) {} .../test.swift:2:7: note: enclosing scope here class Foo { ``` Even though in source-code the enclosing scope (`Foo`) of this guard does not ensure it will always be true. This happens because availability range is propagated by intersecting ranges top-down from the root `TypeRefinementContext`, which is defined by the deployment target, causing the availability range on class `Foo` to become `11.2`. Users may be sharing the same piece of source-code across different projects with their own respective deployment targets which makes such target-dependent warnings confusing at some times, and not-useful at others. We should rather have the warning simply reflect what is in the source. Resolves rdar://77607488
artemcm
force-pushed
the
DeployTargetIndependentAvailabilityDiagnostic
branch
from
655f724 to
5146bd5
Compare
|
@swift-ci please smoke test |
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.
This change separates emission of the "unnecessary availability" diagnostics:
from the deployment target of the current compilation. Instead, these diagnostics will only be emitted if the enclosing scope guard is explicitly specified by the user with an
#availabilityattribute.This fixes cases like the following:
Compiling this with
-target x86_64-apple-macos11.2results in:Even though in source-code the enclosing scope (
Foo) of this guard does not ensure it will always be true.This happens because availability range is propagated by intersecting ranges top-down from the root
TypeRefinementContext, which is defined by the deployment target, causing the availability range on classFooto become11.2.Users may be sharing the same piece of source-code across different projects with their own respective deployment targets which makes such target-dependent warnings confusing at some times, and not-useful at others.
We should rather have the warning simply reflect what is in the source.
Resolves rdar://77607488