This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a7afc9a8843: [-Wcalled-once-parameter] Fix false positives
for cleanup attr (authored by vsavchenko).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98694/new/
https://reviews.llvm.org/D98694
Files:
clang/lib/Analysis/CalledOnceCheck.cpp
clang/test/SemaObjC/warn-called-once.m
Index: clang/test/SemaObjC/warn-called-once.m
===================================================================
--- clang/test/SemaObjC/warn-called-once.m
+++ clang/test/SemaObjC/warn-called-once.m
@@ -1193,4 +1193,46 @@
escape(handler);
}
+// rdar://74441906
+typedef void (^DeferredBlock)(void);
+static inline void DefferedCallback(DeferredBlock *inBlock) { (*inBlock)(); }
+#define _DEFERCONCAT(a, b) a##b
+#define _DEFERNAME(a) _DEFERCONCAT(__DeferredVar_, a)
+#define DEFER __extension__ __attribute__((cleanup(DefferedCallback), unused))
\
+ DeferredBlock _DEFERNAME(__COUNTER__) = ^
+
+- (void)test_cleanup_1:(int)cond
+ withCompletion:(void (^)(void))handler {
+ int error = 0;
+ DEFER {
+ if (error)
+ handler();
+ };
+
+ if (cond) {
+ error = 1;
+ } else {
+ // no-warning
+ handler();
+ }
+}
+
+- (void)test_cleanup_2:(int)cond
+ withCompletion:(void (^)(void))handler {
+ int error = 0;
+ DEFER {
+ if (error)
+ handler();
+ };
+
+ if (cond) {
+ error = 1;
+ } else {
+ handler(); // expected-note{{previous call is here}}
+ }
+
+ // We still can warn about double call even in this case.
+ handler(); // expected-warning{{completion handler is called twice}}
+}
+
@end
Index: clang/lib/Analysis/CalledOnceCheck.cpp
===================================================================
--- clang/lib/Analysis/CalledOnceCheck.cpp
+++ clang/lib/Analysis/CalledOnceCheck.cpp
@@ -812,8 +812,12 @@
}
}
- // Early exit if we don't have parameters for extra analysis.
- if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none())
+ // Early exit if we don't have parameters for extra analysis...
+ if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none() &&
+ // ... or if we've seen variables with cleanup functions.
+ // We can't reason that we've seen every path in this case,
+ // and thus abandon reporting any warnings that imply that.
+ !FunctionHasCleanupVars)
return;
// We are looking for a pair of blocks A, B so that the following is true:
@@ -1601,6 +1605,10 @@
if (Var->getInit()) {
checkEscapee(Var->getInit());
}
+
+ if (Var->hasAttr<CleanupAttr>()) {
+ FunctionHasCleanupVars = true;
+ }
}
}
}
@@ -1669,6 +1677,13 @@
// around.
bool SuppressOnConventionalErrorPaths = false;
+ // The user can annotate variable declarations with cleanup functions, which
+ // essentially imposes a custom destructor logic on that variable.
+ // It is possible to use it, however, to call tracked parameters on all exits
+ // from the function. For this reason, we track the fact that the function
+ // actually has these.
+ bool FunctionHasCleanupVars = false;
+
State CurrentState;
ParamSizedVector<const ParmVarDecl *> TrackedParams;
CFGSizedVector<State> States;
Index: clang/test/SemaObjC/warn-called-once.m
===================================================================
--- clang/test/SemaObjC/warn-called-once.m
+++ clang/test/SemaObjC/warn-called-once.m
@@ -1193,4 +1193,46 @@
escape(handler);
}
+// rdar://74441906
+typedef void (^DeferredBlock)(void);
+static inline void DefferedCallback(DeferredBlock *inBlock) { (*inBlock)(); }
+#define _DEFERCONCAT(a, b) a##b
+#define _DEFERNAME(a) _DEFERCONCAT(__DeferredVar_, a)
+#define DEFER __extension__ __attribute__((cleanup(DefferedCallback), unused)) \
+ DeferredBlock _DEFERNAME(__COUNTER__) = ^
+
+- (void)test_cleanup_1:(int)cond
+ withCompletion:(void (^)(void))handler {
+ int error = 0;
+ DEFER {
+ if (error)
+ handler();
+ };
+
+ if (cond) {
+ error = 1;
+ } else {
+ // no-warning
+ handler();
+ }
+}
+
+- (void)test_cleanup_2:(int)cond
+ withCompletion:(void (^)(void))handler {
+ int error = 0;
+ DEFER {
+ if (error)
+ handler();
+ };
+
+ if (cond) {
+ error = 1;
+ } else {
+ handler(); // expected-note{{previous call is here}}
+ }
+
+ // We still can warn about double call even in this case.
+ handler(); // expected-warning{{completion handler is called twice}}
+}
+
@end
Index: clang/lib/Analysis/CalledOnceCheck.cpp
===================================================================
--- clang/lib/Analysis/CalledOnceCheck.cpp
+++ clang/lib/Analysis/CalledOnceCheck.cpp
@@ -812,8 +812,12 @@
}
}
- // Early exit if we don't have parameters for extra analysis.
- if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none())
+ // Early exit if we don't have parameters for extra analysis...
+ if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none() &&
+ // ... or if we've seen variables with cleanup functions.
+ // We can't reason that we've seen every path in this case,
+ // and thus abandon reporting any warnings that imply that.
+ !FunctionHasCleanupVars)
return;
// We are looking for a pair of blocks A, B so that the following is true:
@@ -1601,6 +1605,10 @@
if (Var->getInit()) {
checkEscapee(Var->getInit());
}
+
+ if (Var->hasAttr<CleanupAttr>()) {
+ FunctionHasCleanupVars = true;
+ }
}
}
}
@@ -1669,6 +1677,13 @@
// around.
bool SuppressOnConventionalErrorPaths = false;
+ // The user can annotate variable declarations with cleanup functions, which
+ // essentially imposes a custom destructor logic on that variable.
+ // It is possible to use it, however, to call tracked parameters on all exits
+ // from the function. For this reason, we track the fact that the function
+ // actually has these.
+ bool FunctionHasCleanupVars = false;
+
State CurrentState;
ParamSizedVector<const ParmVarDecl *> TrackedParams;
CFGSizedVector<State> States;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
