logan-5 created this revision.
logan-5 added reviewers: erik.pilkington, rsmith, arphaman.
logan-5 added a project: clang.
logan-5 requested review of this revision.
Herald added a subscriber: cfe-commits.
Always search the full function scope context if a potential availability
violation is encountered. This fixes both
https://bugs.llvm.org/show_bug.cgi?id=50309 and
https://bugs.llvm.org/show_bug.cgi?id=50310.
Previously, lambdas inside functions would mark their own bodies for later
analysis when encountering a potentially unavailable decl, without taking into
consideration that the entire lambda itself might be correctly guarded inside
an `@available` check. The same applied to inner class member functions. Blocks
happened to work as expected already, since `Sema::getEnclosingFunction()`
skips through block scopes.
This patch instead simply and conservatively marks the entire outermost
function scope for search, and removes some special-case logic that prevented
`DiagnoseUnguardedAvailabilityViolations` from traversing down into lambdas and
nested functions. This correctly accounts for arbitrarily nested lambdas, inner
classes, and blocks that may be inside appropriate `@available` checks at any
ancestor level. It also treats all potential availability violations inside
functions consistently, without being overly sensitive to the current
DeclContext, which previously caused issues where e.g. nested struct members
were warned about twice.
`DiagnoseUnguardedAvailabilityViolations` now has more work to do in some
cases, particularly in functions with many (possibly deeply) nested lambdas and
classes, but the big-O is the same, and the simplicity of the approach and the
fact that it fixes at least two bugs feels like a strong win IMO.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D102338
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaAvailability.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/unguarded-availability.m
Index: clang/test/SemaObjC/unguarded-availability.m
===================================================================
--- clang/test/SemaObjC/unguarded-availability.m
+++ clang/test/SemaObjC/unguarded-availability.m
@@ -125,6 +125,14 @@
(void) ^{
func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
};
+
+ if (@available(macos 10.12, *))
+ (void) ^{
+ func_10_12();
+ (void) ^{
+ func_10_12();
+ };
+ };
}
void test_params(int_10_12 x); // expected-warning {{'int_10_12' is only available on macOS 10.12 or newer}} expected-note{{annotate 'test_params' with an availability attribute to silence this warning}}
@@ -233,6 +241,36 @@
(void)(^ {
func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
});
+
+ if (@available(macos 10.12, *)) {
+ void([]() {
+ func_10_12();
+ void([] () {
+ func_10_12();
+ });
+ struct DontWarn {
+ void f() {
+ func_10_12();
+ }
+ };
+ });
+ }
+
+ if (@available(macos 10.12, *)) {
+ struct DontWarn {
+ void f() {
+ func_10_12();
+ void([] () {
+ func_10_12();
+ });
+ struct DontWarn2 {
+ void f() {
+ func_10_12();
+ }
+ };
+ }
+ };
+ }
}
#endif
@@ -259,9 +297,14 @@
@end
void with_local_struct() {
- struct local { // expected-note{{annotate 'local' with an availability attribute}}
- new_int x; // expected-warning{{'new_int' is only available}}
+ struct local {
+ new_int x; // expected-warning{{'new_int' is only available}} expected-note{{enclose 'new_int' in an @available check}}
};
+ if (@available(macos 10.12, *)) {
+ struct DontWarn {
+ new_int x;
+ };
+ }
}
// rdar://33156429:
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19653,12 +19653,10 @@
if (Spec != AvailSpecs.end())
Version = Spec->getVersion();
- // The use of `@available` in the enclosing function should be analyzed to
+ // The use of `@available` in the enclosing context should be analyzed to
// warn when it's used inappropriately (i.e. not if(@available)).
- if (getCurFunctionOrMethodDecl())
- getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
- else if (getCurBlock() || getCurLambda())
- getCurFunction()->HasPotentialAvailabilityViolations = true;
+ if (FunctionScopeInfo *Context = getCurFunctionAvailabilityContext())
+ Context->HasPotentialAvailabilityViolations = true;
return new (Context)
ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
Index: clang/lib/Sema/SemaAvailability.cpp
===================================================================
--- clang/lib/Sema/SemaAvailability.cpp
+++ clang/lib/Sema/SemaAvailability.cpp
@@ -666,13 +666,6 @@
SemaRef.Context.getTargetInfo().getPlatformMinVersion());
}
- bool TraverseDecl(Decl *D) {
- // Avoid visiting nested functions to prevent duplicate warnings.
- if (!D || isa<FunctionDecl>(D))
- return true;
- return Base::TraverseDecl(D);
- }
-
bool TraverseStmt(Stmt *S) {
if (!S)
return true;
@@ -686,8 +679,6 @@
bool TraverseIfStmt(IfStmt *If);
- bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
-
// for 'case X:' statements, don't bother looking at the 'X'; it can't lead
// to any useful diagnostics.
bool TraverseCaseStmt(CaseStmt *CS) { return TraverseStmt(CS->getSubStmt()); }
@@ -919,6 +910,17 @@
DiagnoseUnguardedAvailability(*this, D).IssueDiagnostics(Body);
}
+FunctionScopeInfo *Sema::getCurFunctionAvailabilityContext() {
+ if (FunctionScopes.empty())
+ return nullptr;
+
+ // Conservatively search the entire current function scope context for
+ // availability violations. This ensures we always correctly analyze nested
+ // classes, blocks, lambdas, etc. that may or may not be inside if(@available)
+ // checks themselves.
+ return FunctionScopes.front();
+}
+
void Sema::DiagnoseAvailabilityOfDecl(NamedDecl *D,
ArrayRef<SourceLocation> Locs,
const ObjCInterfaceDecl *UnknownObjCClass,
@@ -941,11 +943,8 @@
// We need to know the @available context in the current function to
// diagnose this use, let DiagnoseUnguardedAvailabilityViolations do that
// when we're done parsing the current function.
- if (getCurFunctionOrMethodDecl()) {
- getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
- return;
- } else if (getCurBlock() || getCurLambda()) {
- getCurFunction()->HasPotentialAvailabilityViolations = true;
+ if (FunctionScopeInfo *Context = getCurFunctionAvailabilityContext()) {
+ Context->HasPotentialAvailabilityViolations = true;
return;
}
}
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1882,6 +1882,10 @@
/// Retrieve the current captured region, if any.
sema::CapturedRegionScopeInfo *getCurCapturedRegion();
+ /// Retrieve the current function, if any, that should be analyzed for
+ /// potential availability violations.
+ sema::FunctionScopeInfo *getCurFunctionAvailabilityContext();
+
/// WeakTopLevelDeclDecls - access to \#pragma weak-generated Decls
SmallVectorImpl<Decl *> &WeakTopLevelDecls() { return WeakTopLevelDecl; }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
