[webkit.UncountedLambdaCapturesChecker] Add the support for WTF::ScopeExit and WTF::makeVisitor #161926
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
…eExit and WTF::makeVisitor Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesLambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker. Full diff: https://github.com/llvm/llvm-project/pull/161926.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 033eb8cc299b0..460d9dbf1af5e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -50,7 +50,9 @@ class RawPtrRefLambdaCapturesChecker
llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
llvm::DenseSet<const ValueDecl *> ProtectedThisDecls;
+ llvm::DenseSet<const CallExpr *> CallToIgnore;
llvm::DenseSet<const CXXConstructExpr *> ConstructToIgnore;
+ llvm::DenseMap<const VarDecl*, const LambdaExpr*> LambdaOwnerMap;
QualType ClsType;
@@ -101,10 +103,60 @@ class RawPtrRefLambdaCapturesChecker
auto *Init = VD->getInit();
if (!Init)
return true;
- auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
- if (!L)
+ if (auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts())) {
+ LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+ return true;
+ }
+ if (!VD->hasLocalStorage())
return true;
- LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+ if (auto *E = dyn_cast<ExprWithCleanups>(Init))
+ Init = E->getSubExpr();
+ if (auto *E = dyn_cast<CXXBindTemporaryExpr>(Init))
+ Init = E->getSubExpr();
+ if (auto *CE = dyn_cast<CallExpr>(Init)) {
+ if (auto *Callee = CE->getDirectCallee()) {
+ auto FnName = safeGetName(Callee);
+ unsigned ArgCnt = CE->getNumArgs();
+ if (FnName == "makeScopeExit" && ArgCnt == 1) {
+ auto *Arg = CE->getArg(0);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ CallToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ } else if (FnName == "makeVisitor") {
+ for (unsigned ArgIndex = 0; ArgIndex < ArgCnt; ++ArgIndex) {
+ auto *Arg = CE->getArg(ArgIndex);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ CallToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ }
+ }
+ }
+ } else if (auto *CE = dyn_cast<CXXConstructExpr>(Init)) {
+ if (auto *Ctor = CE->getConstructor()) {
+ if (auto *Cls = Ctor->getParent()) {
+ auto FnName = safeGetName(Cls);
+ unsigned ArgCnt = CE->getNumArgs();
+ if (FnName == "ScopeExit" && ArgCnt == 1) {
+ auto *Arg = CE->getArg(0);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ ConstructToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ }
+ }
+ }
+ }
return true;
}
@@ -114,6 +166,12 @@ class RawPtrRefLambdaCapturesChecker
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
if (!VD)
return true;
+ if (auto It = LambdaOwnerMap.find(VD); It != LambdaOwnerMap.end()) {
+ auto *L = It->second;
+ Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
+ ClsType);
+ return true;
+ }
auto *Init = VD->getInit();
if (!Init)
return true;
@@ -167,9 +225,30 @@ class RawPtrRefLambdaCapturesChecker
}
bool VisitCallExpr(CallExpr *CE) override {
+ if (CallToIgnore.contains(CE))
+ return true;
checkCalleeLambda(CE);
- if (auto *Callee = CE->getDirectCallee())
+ if (auto *Callee = CE->getDirectCallee()) {
+ if (auto *Ns = Callee->getParent()) {
+ auto NsName = safeGetName(Ns);
+ bool IsVisitFn = safeGetName(Callee) == "visit";
+ bool ArgCnt = CE->getNumArgs();
+ if (IsVisitFn && ArgCnt && (NsName == "WTF" || NsName == "std")) {
+ if (auto *Arg = CE->getArg(0)) {
+ Arg = Arg->IgnoreParenCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+ if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+ if (LambdaOwnerMap.contains(VD)) {
+ DeclRefExprsToIgnore.insert(DRE);
+ return true;
+ }
+ }
+ }
+ }
+ }
+ }
checkParameters(CE, Callee);
+ }
else if (auto *CalleeE = CE->getCallee()) {
if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
@@ -280,7 +359,7 @@ class RawPtrRefLambdaCapturesChecker
LambdasToIgnore.insert(L);
}
- bool hasProtectedThis(LambdaExpr *L) {
+ bool hasProtectedThis(const LambdaExpr *L) {
for (const LambdaCapture &OtherCapture : L->captures()) {
if (!OtherCapture.capturesVariable())
continue;
@@ -378,8 +457,9 @@ class RawPtrRefLambdaCapturesChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
- void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
- bool ignoreParamVarDecl = false) const {
+ void visitLambdaExpr(const LambdaExpr *L, bool shouldCheckThis,
+ const QualType T,
+ bool ignoreParamVarDecl = false) const {
if (TFA.isTrivial(L->getBody()))
return;
for (const LambdaCapture &C : L->captures()) {
@@ -410,7 +490,7 @@ class RawPtrRefLambdaCapturesChecker
}
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const QualType T, LambdaExpr *L) const {
+ const QualType T, const LambdaExpr *L) const {
assert(CapturedVar);
auto Location = Capture.getLocation();
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index a4ad741182f56..fd1eecdda64fd 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -107,6 +107,79 @@ class HashMap {
ValueType* m_table { nullptr };
};
+class ScopeExit final {
+public:
+ template<typename ExitFunctionParameter>
+ explicit ScopeExit(ExitFunctionParameter&& exitFunction)
+ : m_exitFunction(std::move(exitFunction)) {
+ }
+
+ ScopeExit(ScopeExit&& other)
+ : m_exitFunction(std::move(other.m_exitFunction))
+ , m_execute(std::move(other.m_execute)) {
+ }
+
+ ~ScopeExit() {
+ if (m_execute)
+ m_exitFunction();
+ }
+
+ WTF::Function<void()> take() {
+ m_execute = false;
+ return std::move(m_exitFunction);
+ }
+
+ void release() { m_execute = false; }
+
+ ScopeExit(const ScopeExit&) = delete;
+ ScopeExit& operator=(const ScopeExit&) = delete;
+ ScopeExit& operator=(ScopeExit&&) = delete;
+
+private:
+ WTF::Function<void()> m_exitFunction;
+ bool m_execute { true };
+};
+
+template<typename ExitFunction> ScopeExit makeScopeExit(ExitFunction&&);
+template<typename ExitFunction>
+ScopeExit makeScopeExit(ExitFunction&& exitFunction)
+{
+ return ScopeExit(std::move(exitFunction));
+}
+
+// Visitor adapted from http://stackoverflow.com/questions/25338795/is-there-a-name-for-this-tuple-creation-idiom
+
+template<class A, class... B> struct Visitor : Visitor<A>, Visitor<B...> {
+ Visitor(A a, B... b)
+ : Visitor<A>(a)
+ , Visitor<B...>(b...)
+ {
+ }
+
+ using Visitor<A>::operator ();
+ using Visitor<B...>::operator ();
+};
+
+template<class A> struct Visitor<A> : A {
+ Visitor(A a)
+ : A(a)
+ {
+ }
+
+ using A::operator();
+};
+
+template<class... F> Visitor<F...> makeVisitor(F... f)
+{
+ return Visitor<F...>(f...);
+}
+
+void opaqueFunction();
+template <typename Visitor, typename... Variants> void visit(Visitor&& v, Variants&&... values)
+{
+ opaqueFunction();
+}
+
} // namespace WTF
struct A {
@@ -501,3 +574,81 @@ void RefCountedObj::call() const
};
callLambda(lambda);
}
+
+void scope_exit(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ });
+ someFunction();
+ WTF::ScopeExit scope2([&] {
+ obj->method();
+ });
+ someFunction();
+}
+
+void doWhateverWith(WTF::ScopeExit& obj);
+
+void scope_exit_with_side_effect(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ doWhateverWith(scope);
+}
+
+void scope_exit_static(RefCountable* obj) {
+ static auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+}
+
+WTF::Function<void()> scope_exit_take_lambda(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ return scope.take();
+}
+
+// FIXME: Ideally, we treat release() as a trivial function.
+void scope_exit_release(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ scope.release();
+}
+
+void make_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ });
+}
+
+void use_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ });
+ WTF::visit(visitor, obj);
+}
+
+template <typename Visitor, typename ObjectType>
+void bad_visit(Visitor&, ObjectType*) {
+ someFunction();
+}
+
+void static_visitor(RefCountable* obj) {
+ static auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+}
+
+void bad_use_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ bad_visit(visitor, obj);
+}
|
Member
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesLambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker. Full diff: https://github.com/llvm/llvm-project/pull/161926.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 033eb8cc299b0..460d9dbf1af5e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -50,7 +50,9 @@ class RawPtrRefLambdaCapturesChecker
llvm::DenseSet<const DeclRefExpr *> DeclRefExprsToIgnore;
llvm::DenseSet<const LambdaExpr *> LambdasToIgnore;
llvm::DenseSet<const ValueDecl *> ProtectedThisDecls;
+ llvm::DenseSet<const CallExpr *> CallToIgnore;
llvm::DenseSet<const CXXConstructExpr *> ConstructToIgnore;
+ llvm::DenseMap<const VarDecl*, const LambdaExpr*> LambdaOwnerMap;
QualType ClsType;
@@ -101,10 +103,60 @@ class RawPtrRefLambdaCapturesChecker
auto *Init = VD->getInit();
if (!Init)
return true;
- auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts());
- if (!L)
+ if (auto *L = dyn_cast_or_null<LambdaExpr>(Init->IgnoreParenCasts())) {
+ LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+ return true;
+ }
+ if (!VD->hasLocalStorage())
return true;
- LambdasToIgnore.insert(L); // Evaluate lambdas in VisitDeclRefExpr.
+ if (auto *E = dyn_cast<ExprWithCleanups>(Init))
+ Init = E->getSubExpr();
+ if (auto *E = dyn_cast<CXXBindTemporaryExpr>(Init))
+ Init = E->getSubExpr();
+ if (auto *CE = dyn_cast<CallExpr>(Init)) {
+ if (auto *Callee = CE->getDirectCallee()) {
+ auto FnName = safeGetName(Callee);
+ unsigned ArgCnt = CE->getNumArgs();
+ if (FnName == "makeScopeExit" && ArgCnt == 1) {
+ auto *Arg = CE->getArg(0);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ CallToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ } else if (FnName == "makeVisitor") {
+ for (unsigned ArgIndex = 0; ArgIndex < ArgCnt; ++ArgIndex) {
+ auto *Arg = CE->getArg(ArgIndex);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ CallToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ }
+ }
+ }
+ } else if (auto *CE = dyn_cast<CXXConstructExpr>(Init)) {
+ if (auto *Ctor = CE->getConstructor()) {
+ if (auto *Cls = Ctor->getParent()) {
+ auto FnName = safeGetName(Cls);
+ unsigned ArgCnt = CE->getNumArgs();
+ if (FnName == "ScopeExit" && ArgCnt == 1) {
+ auto *Arg = CE->getArg(0);
+ if (auto *E = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ Arg = E->getSubExpr();
+ if (auto *L = dyn_cast<LambdaExpr>(Arg)) {
+ LambdaOwnerMap.insert(std::make_pair(VD, L));
+ ConstructToIgnore.insert(CE);
+ LambdasToIgnore.insert(L);
+ }
+ }
+ }
+ }
+ }
return true;
}
@@ -114,6 +166,12 @@ class RawPtrRefLambdaCapturesChecker
auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl());
if (!VD)
return true;
+ if (auto It = LambdaOwnerMap.find(VD); It != LambdaOwnerMap.end()) {
+ auto *L = It->second;
+ Checker->visitLambdaExpr(L, shouldCheckThis() && !hasProtectedThis(L),
+ ClsType);
+ return true;
+ }
auto *Init = VD->getInit();
if (!Init)
return true;
@@ -167,9 +225,30 @@ class RawPtrRefLambdaCapturesChecker
}
bool VisitCallExpr(CallExpr *CE) override {
+ if (CallToIgnore.contains(CE))
+ return true;
checkCalleeLambda(CE);
- if (auto *Callee = CE->getDirectCallee())
+ if (auto *Callee = CE->getDirectCallee()) {
+ if (auto *Ns = Callee->getParent()) {
+ auto NsName = safeGetName(Ns);
+ bool IsVisitFn = safeGetName(Callee) == "visit";
+ bool ArgCnt = CE->getNumArgs();
+ if (IsVisitFn && ArgCnt && (NsName == "WTF" || NsName == "std")) {
+ if (auto *Arg = CE->getArg(0)) {
+ Arg = Arg->IgnoreParenCasts();
+ if (auto *DRE = dyn_cast<DeclRefExpr>(Arg)) {
+ if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+ if (LambdaOwnerMap.contains(VD)) {
+ DeclRefExprsToIgnore.insert(DRE);
+ return true;
+ }
+ }
+ }
+ }
+ }
+ }
checkParameters(CE, Callee);
+ }
else if (auto *CalleeE = CE->getCallee()) {
if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
@@ -280,7 +359,7 @@ class RawPtrRefLambdaCapturesChecker
LambdasToIgnore.insert(L);
}
- bool hasProtectedThis(LambdaExpr *L) {
+ bool hasProtectedThis(const LambdaExpr *L) {
for (const LambdaCapture &OtherCapture : L->captures()) {
if (!OtherCapture.capturesVariable())
continue;
@@ -378,8 +457,9 @@ class RawPtrRefLambdaCapturesChecker
visitor.TraverseDecl(const_cast<TranslationUnitDecl *>(TUD));
}
- void visitLambdaExpr(LambdaExpr *L, bool shouldCheckThis, const QualType T,
- bool ignoreParamVarDecl = false) const {
+ void visitLambdaExpr(const LambdaExpr *L, bool shouldCheckThis,
+ const QualType T,
+ bool ignoreParamVarDecl = false) const {
if (TFA.isTrivial(L->getBody()))
return;
for (const LambdaCapture &C : L->captures()) {
@@ -410,7 +490,7 @@ class RawPtrRefLambdaCapturesChecker
}
void reportBug(const LambdaCapture &Capture, ValueDecl *CapturedVar,
- const QualType T, LambdaExpr *L) const {
+ const QualType T, const LambdaExpr *L) const {
assert(CapturedVar);
auto Location = Capture.getLocation();
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index a4ad741182f56..fd1eecdda64fd 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -107,6 +107,79 @@ class HashMap {
ValueType* m_table { nullptr };
};
+class ScopeExit final {
+public:
+ template<typename ExitFunctionParameter>
+ explicit ScopeExit(ExitFunctionParameter&& exitFunction)
+ : m_exitFunction(std::move(exitFunction)) {
+ }
+
+ ScopeExit(ScopeExit&& other)
+ : m_exitFunction(std::move(other.m_exitFunction))
+ , m_execute(std::move(other.m_execute)) {
+ }
+
+ ~ScopeExit() {
+ if (m_execute)
+ m_exitFunction();
+ }
+
+ WTF::Function<void()> take() {
+ m_execute = false;
+ return std::move(m_exitFunction);
+ }
+
+ void release() { m_execute = false; }
+
+ ScopeExit(const ScopeExit&) = delete;
+ ScopeExit& operator=(const ScopeExit&) = delete;
+ ScopeExit& operator=(ScopeExit&&) = delete;
+
+private:
+ WTF::Function<void()> m_exitFunction;
+ bool m_execute { true };
+};
+
+template<typename ExitFunction> ScopeExit makeScopeExit(ExitFunction&&);
+template<typename ExitFunction>
+ScopeExit makeScopeExit(ExitFunction&& exitFunction)
+{
+ return ScopeExit(std::move(exitFunction));
+}
+
+// Visitor adapted from http://stackoverflow.com/questions/25338795/is-there-a-name-for-this-tuple-creation-idiom
+
+template<class A, class... B> struct Visitor : Visitor<A>, Visitor<B...> {
+ Visitor(A a, B... b)
+ : Visitor<A>(a)
+ , Visitor<B...>(b...)
+ {
+ }
+
+ using Visitor<A>::operator ();
+ using Visitor<B...>::operator ();
+};
+
+template<class A> struct Visitor<A> : A {
+ Visitor(A a)
+ : A(a)
+ {
+ }
+
+ using A::operator();
+};
+
+template<class... F> Visitor<F...> makeVisitor(F... f)
+{
+ return Visitor<F...>(f...);
+}
+
+void opaqueFunction();
+template <typename Visitor, typename... Variants> void visit(Visitor&& v, Variants&&... values)
+{
+ opaqueFunction();
+}
+
} // namespace WTF
struct A {
@@ -501,3 +574,81 @@ void RefCountedObj::call() const
};
callLambda(lambda);
}
+
+void scope_exit(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ });
+ someFunction();
+ WTF::ScopeExit scope2([&] {
+ obj->method();
+ });
+ someFunction();
+}
+
+void doWhateverWith(WTF::ScopeExit& obj);
+
+void scope_exit_with_side_effect(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ doWhateverWith(scope);
+}
+
+void scope_exit_static(RefCountable* obj) {
+ static auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+}
+
+WTF::Function<void()> scope_exit_take_lambda(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ return scope.take();
+}
+
+// FIXME: Ideally, we treat release() as a trivial function.
+void scope_exit_release(RefCountable* obj) {
+ auto scope = WTF::makeScopeExit([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ scope.release();
+}
+
+void make_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ });
+}
+
+void use_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ });
+ WTF::visit(visitor, obj);
+}
+
+template <typename Visitor, typename ObjectType>
+void bad_visit(Visitor&, ObjectType*) {
+ someFunction();
+}
+
+void static_visitor(RefCountable* obj) {
+ static auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+}
+
+void bad_use_visitor(RefCountable* obj) {
+ auto visitor = WTF::makeVisitor([&] {
+ obj->method();
+ // expected-warning@-1{{Implicitly captured raw-pointer 'obj' to uncounted type is unsafe [webkit.UncountedLambdaCapturesChecker]}}
+ });
+ bad_visit(visitor, obj);
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
t-rasmud
approved these changes
Oct 23, 2025
Contributor
t-rasmud
left a comment
t-rasmud
left a comment
There was a problem hiding this comment.
LGTM! I left an optional nit. Thank you for writing extensive test cases.
| checkCalleeLambda(CE); | ||
| if (auto *Callee = CE->getDirectCallee()) | ||
| if (auto *Callee = CE->getDirectCallee()) { |
Contributor
There was a problem hiding this comment.
Optional Nit: Modify the code to use early returns.
Contributor
Author
There was a problem hiding this comment.
I don't think we can quite convert to early returns since we to handle else-if case when getDirectCallee returns nullptr. But I'd extract a helper function isVisitFunction so that more early returns could be used inside there.
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Oct 24, 2025
…eExit and WTF::makeVisitor (llvm#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker. (cherry picked from commit c07d305)
dvbuka
pushed a commit
to dvbuka/llvm-project
that referenced
this pull request
Oct 27, 2025
…eExit and WTF::makeVisitor (llvm#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker.
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
…eExit and WTF::makeVisitor (llvm#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker.
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
…eExit and WTF::makeVisitor (llvm#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker.
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Nov 5, 2025
…eExit and WTF::makeVisitor (llvm#161926) Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope. Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker. (cherry picked from commit c07d305)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Lambda passed to WTF::ScopeExit / WTF::makeScopeExit and WTF::makeVisitor should be ignored by the lambda captures checker so long as its resulting object doesn't escape the current scope.
Unfortunately, recognizing this pattern generally is too hard to do so directly hard-code these two function names to the checker.