[clang] a5a3efa - [Sema] Always search the full function scope context if a potential availability violation is encountered
Logan Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon May 24 21:26:39 PDT 2021
Author: Logan Smith
Date: 2021-05-24T21:13:30-07:00
New Revision: a5a3efa82a77ab7a1c9787ef97b547a4a81f2440
URL: https://github.com/llvm/llvm-project/commit/a5a3efa82a77ab7a1c9787ef97b547a4a81f2440
DIFF: https://github.com/llvm/llvm-project/commit/a5a3efa82a77ab7a1c9787ef97b547a4a81f2440.diff
LOG: [Sema] 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.
Differential Revision: https://reviews.llvm.org/D102338
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaAvailability.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/unguarded-availability.m
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ac6471c4f04a..706293fa929ce 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1882,6 +1882,10 @@ class Sema final {
/// 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; }
diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index 74c4b9e16f74e..d69600aaf94ea 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -666,13 +666,6 @@ class DiagnoseUnguardedAvailability
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 @@ class DiagnoseUnguardedAvailability
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 @@ void Sema::DiagnoseUnguardedAvailabilityViolations(Decl *D) {
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 @@ void Sema::DiagnoseAvailabilityOfDecl(NamedDecl *D,
// 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;
}
}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2cabd7ddeab5a..5361eb37fafce 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19650,12 +19650,10 @@ ExprResult Sema::ActOnObjCAvailabilityCheckExpr(
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);
diff --git a/clang/test/SemaObjC/unguarded-availability.m b/clang/test/SemaObjC/unguarded-availability.m
index c185a363cd688..0ee57301c3f54 100644
--- a/clang/test/SemaObjC/unguarded-availability.m
+++ b/clang/test/SemaObjC/unguarded-availability.m
@@ -125,6 +125,14 @@ void test_blocks() {
(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 f() {
(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 @@ @interface Proper // expected-note{{annotate 'Proper' with an availability attri
@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:
More information about the cfe-commits
mailing list