[webkit.RefCntblBaseVirtualDtor] Make ThreadSafeRefCounted not generate warnings #107676
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Ryosuke Niwa (rniwa) ChangesThis PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning for ThreadSafeRefCounted when the destruction thread is a specific thread. Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type. This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to an "opaque" function; i.e. a function without definition / body. Full diff: https://github.com/llvm/llvm-project/pull/107676.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 26879f2f87c8be..8e97efc1ab8e8e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
#include <optional>
@@ -67,6 +68,15 @@ class DerefFuncDeleteExprVisitor
const Decl *D = CE->getCalleeDecl();
if (D && D->hasBody())
return VisitBody(D->getBody());
+ else if (!VisitLambdaBody) {
+ for (unsigned i = 0; i < CE->getNumArgs(); ++i) {
+ auto *Arg = CE->getArg(i);
+ VisitLambdaBody = true;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = false; });
+ if (VisitChildren(Arg))
+ return true;
+ }
+ }
return false;
}
@@ -113,12 +123,22 @@ class DerefFuncDeleteExprVisitor
// Return false since the contents of lambda isn't necessarily executed.
// If it is executed, VisitCallExpr above will visit its body.
- bool VisitLambdaExpr(const LambdaExpr *) { return false; }
+ // Allow returning true for a lambda if it's a function argument to another
+ // function without body / definition.
+ bool VisitLambdaExpr(const LambdaExpr *E) {
+ if (VisitLambdaBody) {
+ VisitLambdaBody = false;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = true; });
+ return VisitChildren(E);
+ }
+ return false;
+ }
private:
const TemplateArgumentList *ArgList{nullptr};
const CXXRecordDecl *ClassDecl;
llvm::DenseSet<const Stmt *> VisitedBody;
+ bool VisitLambdaBody{false};
};
class RefCntblBaseVirtualDtorChecker
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
new file mode 100644
index 00000000000000..e149c1d3d3575f
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
@@ -0,0 +1,232 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
+
+#include "mock-types.h"
+
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapperBase {
+public:
+ virtual ~CallableWrapperBase() { }
+ virtual Out call(In...) = 0;
+};
+
+template<typename, typename, typename...> class CallableWrapper;
+
+template<typename CallableType, typename Out, typename... In>
+class CallableWrapper : public CallableWrapperBase<Out, In...> {
+public:
+ explicit CallableWrapper(CallableType&& callable)
+ : m_callable(WTFMove(callable)) { }
+ CallableWrapper(const CallableWrapper&) = delete;
+ CallableWrapper& operator=(const CallableWrapper&) = delete;
+ Out call(In... in) final;
+private:
+ CallableType m_callable;
+};
+
+} // namespace Detail
+
+template<typename> class Function;
+
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>*);
+
+template <typename Out, typename... In>
+class Function<Out(In...)> {
+public:
+ using Impl = Detail::CallableWrapperBase<Out, In...>;
+
+ Function() = default;
+
+ template<typename FunctionType>
+ Function(FunctionType f);
+
+ Out operator()(In... in) const;
+ explicit operator bool() const { return !!m_callableWrapper; }
+
+private:
+ enum AdoptTag { Adopt };
+ Function(Impl* impl, AdoptTag)
+ : m_callableWrapper(impl)
+ {
+ }
+
+ friend Function adopt<Out, In...>(Impl*);
+
+ Impl* m_callableWrapper;
+};
+
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>* impl)
+{
+ return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt);
+}
+
+template<typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> Ref<T, PtrTraits, RefDerefTraits> adoptRef(T&);
+
+template<typename T, typename _PtrTraits, typename RefDerefTraits>
+inline Ref<T, _PtrTraits, RefDerefTraits> adoptRef(T& reference)
+{
+ return Ref<T, _PtrTraits, RefDerefTraits>(reference);
+}
+
+enum class DestructionThread : unsigned char { Any, Main, MainRunLoop };
+void ensureOnMainThread(Function<void()>&&); // Sync if called on main thread, async otherwise.
+void ensureOnMainRunLoop(Function<void()>&&); // Sync if called on main run loop, async otherwise.
+
+class ThreadSafeRefCountedBase {
+public:
+ ThreadSafeRefCountedBase() = default;
+
+ void ref() const
+ {
+ ++m_refCount;
+ }
+
+ bool hasOneRef() const
+ {
+ return refCount() == 1;
+ }
+
+ unsigned refCount() const
+ {
+ return m_refCount;
+ }
+
+protected:
+ bool derefBase() const
+ {
+ if (!--m_refCount) {
+ m_refCount = 1;
+ return true;
+ }
+ return false;
+ }
+
+private:
+ mutable unsigned m_refCount { 1 };
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class ThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ if constexpr (destructionThread == DestructionThread::Any) {
+ delete static_cast<const T*>(this);
+ } else if constexpr (destructionThread == DestructionThread::Main) {
+ ensureOnMainThread([this] {
+ delete static_cast<const T*>(this);
+ });
+ }
+ }
+
+protected:
+ ThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass final : public ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass> create()
+ {
+ return adoptRef(*new FancyRefCountedClass());
+ }
+
+ virtual ~FancyRefCountedClass();
+
+private:
+ FancyRefCountedClass();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class BadThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ [this] {
+ delete static_cast<const T*>(this);
+ };
+ }
+
+protected:
+ BadThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass2 final : public ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+// expected-warning@-1{{Class 'ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main>' is used as a base of class 'FancyRefCountedClass2' but doesn't have virtual destructor}}
+public:
+ static Ref<FancyRefCountedClass2> create()
+ {
+ return adoptRef(*new FancyRefCountedClass2());
+ }
+
+ virtual ~FancyRefCountedClass2();
+
+private:
+ FancyRefCountedClass2();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class NestedThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ destroyThis();
+ });
+ }
+
+protected:
+ NestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass3 final : public NestedThreadSafeRefCounted<FancyRefCountedClass3, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass3> create()
+ {
+ return adoptRef(*new FancyRefCountedClass3());
+ }
+
+ virtual ~FancyRefCountedClass3();
+
+private:
+ FancyRefCountedClass3();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class BadNestedThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ });
+ }
+
+protected:
+ BadNestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass4 final : public BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main> {
+// expected-warning@-1{{Class 'BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main>' is used as a base of class 'FancyRefCountedClass4' but doesn't have virtual destructor}}
+public:
+ static Ref<FancyRefCountedClass4> create()
+ {
+ return adoptRef(*new FancyRefCountedClass4());
+ }
+
+ virtual ~FancyRefCountedClass4();
+
+private:
+ FancyRefCountedClass4();
+};
|
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Ryosuke Niwa (rniwa) ChangesThis PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning for ThreadSafeRefCounted when the destruction thread is a specific thread. Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type. This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to an "opaque" function; i.e. a function without definition / body. Full diff: https://github.com/llvm/llvm-project/pull/107676.diff 2 Files Affected:
diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
index 26879f2f87c8be..8e97efc1ab8e8e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
#include <optional>
@@ -67,6 +68,15 @@ class DerefFuncDeleteExprVisitor
const Decl *D = CE->getCalleeDecl();
if (D && D->hasBody())
return VisitBody(D->getBody());
+ else if (!VisitLambdaBody) {
+ for (unsigned i = 0; i < CE->getNumArgs(); ++i) {
+ auto *Arg = CE->getArg(i);
+ VisitLambdaBody = true;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = false; });
+ if (VisitChildren(Arg))
+ return true;
+ }
+ }
return false;
}
@@ -113,12 +123,22 @@ class DerefFuncDeleteExprVisitor
// Return false since the contents of lambda isn't necessarily executed.
// If it is executed, VisitCallExpr above will visit its body.
- bool VisitLambdaExpr(const LambdaExpr *) { return false; }
+ // Allow returning true for a lambda if it's a function argument to another
+ // function without body / definition.
+ bool VisitLambdaExpr(const LambdaExpr *E) {
+ if (VisitLambdaBody) {
+ VisitLambdaBody = false;
+ auto Restore = llvm::make_scope_exit([&] { VisitLambdaBody = true; });
+ return VisitChildren(E);
+ }
+ return false;
+ }
private:
const TemplateArgumentList *ArgList{nullptr};
const CXXRecordDecl *ClassDecl;
llvm::DenseSet<const Stmt *> VisitedBody;
+ bool VisitLambdaBody{false};
};
class RefCntblBaseVirtualDtorChecker
diff --git a/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
new file mode 100644
index 00000000000000..e149c1d3d3575f
--- /dev/null
+++ b/clang/test/Analysis/Checkers/WebKit/ref-cntbl-crtp-base-no-virtual-dtor.cpp
@@ -0,0 +1,232 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.RefCntblBaseVirtualDtor -verify %s
+
+#include "mock-types.h"
+
+namespace Detail {
+
+template<typename Out, typename... In>
+class CallableWrapperBase {
+public:
+ virtual ~CallableWrapperBase() { }
+ virtual Out call(In...) = 0;
+};
+
+template<typename, typename, typename...> class CallableWrapper;
+
+template<typename CallableType, typename Out, typename... In>
+class CallableWrapper : public CallableWrapperBase<Out, In...> {
+public:
+ explicit CallableWrapper(CallableType&& callable)
+ : m_callable(WTFMove(callable)) { }
+ CallableWrapper(const CallableWrapper&) = delete;
+ CallableWrapper& operator=(const CallableWrapper&) = delete;
+ Out call(In... in) final;
+private:
+ CallableType m_callable;
+};
+
+} // namespace Detail
+
+template<typename> class Function;
+
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>*);
+
+template <typename Out, typename... In>
+class Function<Out(In...)> {
+public:
+ using Impl = Detail::CallableWrapperBase<Out, In...>;
+
+ Function() = default;
+
+ template<typename FunctionType>
+ Function(FunctionType f);
+
+ Out operator()(In... in) const;
+ explicit operator bool() const { return !!m_callableWrapper; }
+
+private:
+ enum AdoptTag { Adopt };
+ Function(Impl* impl, AdoptTag)
+ : m_callableWrapper(impl)
+ {
+ }
+
+ friend Function adopt<Out, In...>(Impl*);
+
+ Impl* m_callableWrapper;
+};
+
+template<typename Out, typename... In> Function<Out(In...)> adopt(Detail::CallableWrapperBase<Out, In...>* impl)
+{
+ return Function<Out(In...)>(impl, Function<Out(In...)>::Adopt);
+}
+
+template<typename T, typename PtrTraits = RawPtrTraits<T>, typename RefDerefTraits = DefaultRefDerefTraits<T>> Ref<T, PtrTraits, RefDerefTraits> adoptRef(T&);
+
+template<typename T, typename _PtrTraits, typename RefDerefTraits>
+inline Ref<T, _PtrTraits, RefDerefTraits> adoptRef(T& reference)
+{
+ return Ref<T, _PtrTraits, RefDerefTraits>(reference);
+}
+
+enum class DestructionThread : unsigned char { Any, Main, MainRunLoop };
+void ensureOnMainThread(Function<void()>&&); // Sync if called on main thread, async otherwise.
+void ensureOnMainRunLoop(Function<void()>&&); // Sync if called on main run loop, async otherwise.
+
+class ThreadSafeRefCountedBase {
+public:
+ ThreadSafeRefCountedBase() = default;
+
+ void ref() const
+ {
+ ++m_refCount;
+ }
+
+ bool hasOneRef() const
+ {
+ return refCount() == 1;
+ }
+
+ unsigned refCount() const
+ {
+ return m_refCount;
+ }
+
+protected:
+ bool derefBase() const
+ {
+ if (!--m_refCount) {
+ m_refCount = 1;
+ return true;
+ }
+ return false;
+ }
+
+private:
+ mutable unsigned m_refCount { 1 };
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class ThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ if constexpr (destructionThread == DestructionThread::Any) {
+ delete static_cast<const T*>(this);
+ } else if constexpr (destructionThread == DestructionThread::Main) {
+ ensureOnMainThread([this] {
+ delete static_cast<const T*>(this);
+ });
+ }
+ }
+
+protected:
+ ThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass final : public ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass> create()
+ {
+ return adoptRef(*new FancyRefCountedClass());
+ }
+
+ virtual ~FancyRefCountedClass();
+
+private:
+ FancyRefCountedClass();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class BadThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+
+ [this] {
+ delete static_cast<const T*>(this);
+ };
+ }
+
+protected:
+ BadThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass2 final : public ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main> {
+// expected-warning@-1{{Class 'ThreadSafeRefCounted<FancyRefCountedClass, DestructionThread::Main>' is used as a base of class 'FancyRefCountedClass2' but doesn't have virtual destructor}}
+public:
+ static Ref<FancyRefCountedClass2> create()
+ {
+ return adoptRef(*new FancyRefCountedClass2());
+ }
+
+ virtual ~FancyRefCountedClass2();
+
+private:
+ FancyRefCountedClass2();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class NestedThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ destroyThis();
+ });
+ }
+
+protected:
+ NestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass3 final : public NestedThreadSafeRefCounted<FancyRefCountedClass3, DestructionThread::Main> {
+public:
+ static Ref<FancyRefCountedClass3> create()
+ {
+ return adoptRef(*new FancyRefCountedClass3());
+ }
+
+ virtual ~FancyRefCountedClass3();
+
+private:
+ FancyRefCountedClass3();
+};
+
+template<class T, DestructionThread destructionThread = DestructionThread::Any> class BadNestedThreadSafeRefCounted : public ThreadSafeRefCountedBase {
+public:
+ void deref() const
+ {
+ if (!derefBase())
+ return;
+ ensureOnMainThread([&] {
+ auto destroyThis = [&] {
+ delete static_cast<const T*>(this);
+ };
+ });
+ }
+
+protected:
+ BadNestedThreadSafeRefCounted() = default;
+};
+
+class FancyRefCountedClass4 final : public BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main> {
+// expected-warning@-1{{Class 'BadNestedThreadSafeRefCounted<FancyRefCountedClass4, DestructionThread::Main>' is used as a base of class 'FancyRefCountedClass4' but doesn't have virtual destructor}}
+public:
+ static Ref<FancyRefCountedClass4> create()
+ {
+ return adoptRef(*new FancyRefCountedClass4());
+ }
+
+ virtual ~FancyRefCountedClass4();
+
+private:
+ FancyRefCountedClass4();
+};
|
haoNoQ
reviewed
Sep 10, 2024
clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
Outdated
Show resolved
Hide resolved
Yes, we're not putting any requirement for the functions for now. |
rniwa
force-pushed
the
make-thread-safe-ref-counted-not-generate-warning
branch
from
ab65263 to
be0c2e0
Compare
Actually, why not. We can just require the function names to be ensureOnMainThread or ensureOnMainRunLoop for now. We can relax the requirement further if we find more cases. |
haoNoQ
reviewed
Sep 11, 2024
clang/lib/StaticAnalyzer/Checkers/WebKit/RefCntblBaseVirtualDtorChecker.cpp
Outdated
Show resolved
Hide resolved
rniwa
force-pushed
the
make-thread-safe-ref-counted-not-generate-warning
branch
2 times, most recently
from
cb214cc to
3a5031d
Compare
… warning for ThreadSafeRefCounted when the destruction thread is a specific thread. Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type. This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to ensureOnMainThread or ensureOnMainRunLoop.
rniwa
force-pushed
the
make-thread-safe-ref-counted-not-generate-warning
branch
from
3a5031d to
516ba7d
Compare
rniwa
added a commit
to rniwa/llvm-project
that referenced
this pull request
Feb 3, 2025
…te warnings (llvm#107676) This PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning for ThreadSafeRefCounted when the destruction thread is a specific thread. Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type. This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to an "opaque" function; i.e. a function without definition / body.
devincoughlin
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 25, 2025
…te warnings (llvm#107676) This PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning for ThreadSafeRefCounted when the destruction thread is a specific thread. Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type. This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to an "opaque" function; i.e. a function without definition / body.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR makes WebKit's RefCntblBaseVirtualDtor checker not generate a warning for ThreadSafeRefCounted when the destruction thread is a specific thread.
Prior to this PR, we only allowed CRTP classes without a virtual destructor if its deref function had an explicit cast to the derived type, skipping any lambda declarations which aren't invoked. This ends up generating a warning for ThreadSafeRefCounted when a specific thread is used to destruct the object because there is no inline body / definition for ensureOnMainThread and ensureOnMainRunLoop and DerefFuncDeleteExprVisitor concludes that there is no explicit delete of the derived type.
This PR relaxes the condition DerefFuncDeleteExprVisitor checks by allowing a delete expression to appear within a lambda declaration if it's an argument to an "opaque" function; i.e. a function without definition / body.