Skip to content

Commit 5146bd5

Browse files
artemcmartemcm
committed
Report unnecessary availability warnings independently from the deployment 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
1 parent 6f44ba4 commit 5146bd5

File tree

5 files changed

+68
-25
lines changed

5 files changed

+68
-25
lines changed

‎include/swift/AST/TypeRefinementContext.h‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,21 @@ class TypeRefinementContext {
154154

155155
SourceRange SrcRange;
156156

157+
/// A canonical availiability info for this context, computed top-down from the root
158+
/// context (compilation deployment target).
157159
AvailabilityContext AvailabilityInfo;
158160

161+
/// If this context was annotated with an availability attribute, this property captures that.
162+
/// It differs from the above `AvailabilityInfo` by being independent of the deployment target,
163+
/// and is used for providing availability attribute redundancy warning diagnostics.
164+
AvailabilityContext ExplicitAvailabilityInfo;
165+
159166
std::vector<TypeRefinementContext *> Children;
160167

161168
TypeRefinementContext(ASTContext &Ctx, IntroNode Node,
162169
TypeRefinementContext *Parent, SourceRange SrcRange,
163-
const AvailabilityContext &Info);
170+
const AvailabilityContext &Info,
171+
const AvailabilityContext &ExplicitInfo);
164172

165173
public:
166174

@@ -172,6 +180,7 @@ class TypeRefinementContext {
172180
static TypeRefinementContext *createForDecl(ASTContext &Ctx, Decl *D,
173181
TypeRefinementContext *Parent,
174182
const AvailabilityContext &Info,
183+
const AvailabilityContext &ExplicitInfo,
175184
SourceRange SrcRange);
176185

177186
/// Create a refinement context for the Then branch of the given IfStmt.
@@ -245,6 +254,12 @@ class TypeRefinementContext {
245254
return AvailabilityInfo;
246255
}
247256

257+
/// Returns the information on what availability was specified by the programmer
258+
/// on this context (if any).
259+
const AvailabilityContext &getExplicitAvailabilityInfo() const {
260+
return ExplicitAvailabilityInfo;
261+
}
262+
248263
/// Adds a child refinement context.
249264
void addChild(TypeRefinementContext *Child) {
250265
assert(Child->getSourceRange().isValid());

‎lib/AST/TypeRefinementContext.cpp‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ using namespace swift;
2828
TypeRefinementContext::TypeRefinementContext(ASTContext &Ctx, IntroNode Node,
2929
TypeRefinementContext *Parent,
3030
SourceRange SrcRange,
31-
const AvailabilityContext &Info)
32-
: Node(Node), SrcRange(SrcRange), AvailabilityInfo(Info) {
31+
const AvailabilityContext &Info,
32+
const AvailabilityContext &ExplicitInfo)
33+
: Node(Node), SrcRange(SrcRange),
34+
AvailabilityInfo(Info), ExplicitAvailabilityInfo(ExplicitInfo) {
3335
if (Parent) {
3436
assert(SrcRange.isValid());
3537
Parent->addChild(this);
@@ -46,18 +48,20 @@ TypeRefinementContext::createRoot(SourceFile *SF,
4648
ASTContext &Ctx = SF->getASTContext();
4749
return new (Ctx)
4850
TypeRefinementContext(Ctx, SF,
49-
/*Parent=*/nullptr, SourceRange(), Info);
51+
/*Parent=*/nullptr, SourceRange(),
52+
Info, AvailabilityContext::alwaysAvailable());
5053
}
5154

5255
TypeRefinementContext *
5356
TypeRefinementContext::createForDecl(ASTContext &Ctx, Decl *D,
5457
TypeRefinementContext *Parent,
5558
const AvailabilityContext &Info,
59+
const AvailabilityContext &ExplicitInfo,
5660
SourceRange SrcRange) {
5761
assert(D);
5862
assert(Parent);
5963
return new (Ctx)
60-
TypeRefinementContext(Ctx, D, Parent, SrcRange, Info);
64+
TypeRefinementContext(Ctx, D, Parent, SrcRange, Info, ExplicitInfo);
6165
}
6266

6367
TypeRefinementContext *
@@ -68,7 +72,8 @@ TypeRefinementContext::createForIfStmtThen(ASTContext &Ctx, IfStmt *S,
6872
assert(Parent);
6973
return new (Ctx)
7074
TypeRefinementContext(Ctx, IntroNode(S, /*IsThen=*/true), Parent,
71-
S->getThenStmt()->getSourceRange(), Info);
75+
S->getThenStmt()->getSourceRange(),
76+
Info, /* ExplicitInfo */Info);
7277
}
7378

7479
TypeRefinementContext *
@@ -79,7 +84,8 @@ TypeRefinementContext::createForIfStmtElse(ASTContext &Ctx, IfStmt *S,
7984
assert(Parent);
8085
return new (Ctx)
8186
TypeRefinementContext(Ctx, IntroNode(S, /*IsThen=*/false), Parent,
82-
S->getElseStmt()->getSourceRange(), Info);
87+
S->getElseStmt()->getSourceRange(),
88+
Info, /* ExplicitInfo */Info);
8389
}
8490

8591
TypeRefinementContext *
@@ -92,7 +98,7 @@ TypeRefinementContext::createForConditionFollowingQuery(ASTContext &Ctx,
9298
assert(Parent);
9399
SourceRange Range(PAI->getEndLoc(), LastElement.getEndLoc());
94100
return new (Ctx) TypeRefinementContext(Ctx, PAI, Parent, Range,
95-
Info);
101+
Info, /* ExplicitInfo */Info);
96102
}
97103

98104
TypeRefinementContext *
@@ -107,7 +113,7 @@ TypeRefinementContext::createForGuardStmtFallthrough(ASTContext &Ctx,
107113
SourceRange Range(RS->getEndLoc(), ContainingBraceStmt->getEndLoc());
108114
return new (Ctx) TypeRefinementContext(Ctx,
109115
IntroNode(RS, /*IsFallthrough=*/true),
110-
Parent, Range, Info);
116+
Parent, Range, Info, /* ExplicitInfo */Info);
111117
}
112118

113119
TypeRefinementContext *
@@ -118,7 +124,7 @@ TypeRefinementContext::createForGuardStmtElse(ASTContext &Ctx, GuardStmt *RS,
118124
assert(Parent);
119125
return new (Ctx)
120126
TypeRefinementContext(Ctx, IntroNode(RS, /*IsFallthrough=*/false), Parent,
121-
RS->getBody()->getSourceRange(), Info);
127+
RS->getBody()->getSourceRange(), Info, /* ExplicitInfo */Info);
122128
}
123129

124130
TypeRefinementContext *
@@ -128,7 +134,7 @@ TypeRefinementContext::createForWhileStmtBody(ASTContext &Ctx, WhileStmt *S,
128134
assert(S);
129135
assert(Parent);
130136
return new (Ctx) TypeRefinementContext(
131-
Ctx, S, Parent, S->getBody()->getSourceRange(), Info);
137+
Ctx, S, Parent, S->getBody()->getSourceRange(), Info, /* ExplicitInfo */Info);
132138
}
133139

134140
// Only allow allocation of TypeRefinementContext using the allocator in

‎lib/Sema/TypeCheckAvailability.cpp‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -400,13 +400,15 @@ class TypeRefinementContextBuilder : private ASTWalker {
400400
// The potential versions in the declaration are constrained by both
401401
// the declared availability of the declaration and the potential versions
402402
// of its lexical context.
403-
AvailabilityContext DeclInfo =
403+
AvailabilityContext ExplicitDeclInfo =
404404
swift::AvailabilityInference::availableRange(D, Context);
405+
AvailabilityContext DeclInfo = ExplicitDeclInfo;
405406
DeclInfo.intersectWith(getCurrentTRC()->getAvailabilityInfo());
406407

407408
TypeRefinementContext *NewTRC =
408409
TypeRefinementContext::createForDecl(Context, D, getCurrentTRC(),
409410
DeclInfo,
411+
ExplicitDeclInfo,
410412
refinementSourceRangeForDecl(D));
411413

412414
// Record the TRC for this storage declaration so that
@@ -646,6 +648,8 @@ class TypeRefinementContextBuilder : private ASTWalker {
646648
for (StmtConditionElement Element : Cond) {
647649
TypeRefinementContext *CurrentTRC = getCurrentTRC();
648650
AvailabilityContext CurrentInfo = CurrentTRC->getAvailabilityInfo();
651+
AvailabilityContext CurrentExplicitInfo =
652+
CurrentTRC->getExplicitAvailabilityInfo();
649653

650654
// If the element is not a condition, walk it in the current TRC.
651655
if (Element.getKind() != StmtConditionElement::CK_Availability) {
@@ -708,24 +712,17 @@ class TypeRefinementContextBuilder : private ASTWalker {
708712
continue;
709713
}
710714

711-
712-
// If the version range for the current TRC is completely contained in
713-
// the range for the spec, then a version query can never be false, so the
714-
// spec is useless. If so, report this.
715-
if (CurrentInfo.isContainedIn(NewConstraint)) {
715+
// If the explicitly-specified (via #availability) version range for the
716+
// current TRC is completely contained in the range for the spec, then
717+
// a version query can never be false, so the spec is useless.
718+
// If so, report this.
719+
if (CurrentExplicitInfo.isContainedIn(NewConstraint)) {
716720
DiagnosticEngine &Diags = Context.Diags;
717-
// Some availability checks will always pass because the minimum
718-
// deployment target guarantees they will never be false. We don't
719-
// diagnose these checks as useless because the source file may
720-
// be shared with other projects/targets having older deployment
721-
// targets. We don't currently have a mechanism for the user to
722-
// suppress these warnings (for example, by indicating when the
723-
// required compatibility version is different than the deployment
724-
// target).
725721
if (CurrentTRC->getReason() != TypeRefinementContext::Reason::Root) {
726722
PlatformKind BestPlatform = targetPlatform(Context.LangOpts);
727723
auto *PlatformSpec =
728724
dyn_cast<PlatformVersionConstraintAvailabilitySpec>(Spec);
725+
729726
// If possible, try to report the diagnostic in terms for the
730727
// platform the user uttered in the '#available()'. For a platform
731728
// that inherits availability from another platform it may be
@@ -738,7 +735,9 @@ class TypeRefinementContextBuilder : private ASTWalker {
738735
Diags.diagnose(CurrentTRC->getIntroductionLoc(),
739736
diag::availability_query_useless_enclosing_scope_here);
740737
}
738+
}
741739

740+
if (CurrentInfo.isContainedIn(NewConstraint)) {
742741
// No need to actually create the refinement context if we know it is
743742
// useless.
744743
continue;

‎test/Sema/availability_implicitly_unnecessary.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %target-typecheck-verify-swift -verify -target %target-cpu-apple-macosx11.2 -disable-objc-attr-requires-foundation-module
2+
// REQUIRES: OS=macosx
3+
4+
@available(macOS 11.0, *)
5+
class Foo {
6+
func foo() {
7+
if #available(macOS 11.1, *) {}
8+
}
9+
}

‎test/Sema/availability_unnecessary.swift‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Ensure that the `unnecessary check` availability warning is emitted when unnecessary due to
2+
// scope's explicit annotation
3+
4+
// RUN: %target-typecheck-verify-swift -verify -target %target-cpu-apple-macosx11.2 -disable-objc-attr-requires-foundation-module
5+
// REQUIRES: OS=macosx
6+
7+
@available(macOS 11.1, *)
8+
class Foo {
9+
// expected-note@-1 {{enclosing scope here}}
10+
func foo() {
11+
// expected-warning@+1 {{unnecessary check for 'macOS'; enclosing scope ensures guard will always be true}}
12+
if #available(macOS 11.0, *) {}
13+
}
14+
}

0 commit comments

Comments
 (0)