njames93 updated this revision to Diff 309093.
njames93 added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix assertion due to a pointer stored in a ManagedStatic.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92480/new/
https://reviews.llvm.org/D92480
Files:
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===================================================================
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,10 +70,27 @@
template <class Derived> class RefCountedBase {
mutable unsigned RefCount = 0;
-public:
+protected:
RefCountedBase() = default;
+ // Copy and move constructors/assignments are no-ops as the RefCount isn't
+ // dictated by the class directly.
RefCountedBase(const RefCountedBase &) {}
+ RefCountedBase(RefCountedBase &&) {}
+ RefCountedBase &operator=(const RefCountedBase &) { return *this; }
+ RefCountedBase &operator=(RefCountedBase &&) { return *this; }
+
+#ifndef NDEBUG
+ ~RefCountedBase() {
+ assert(RefCount == 0 &&
+ "Destruction occured when there are still references to this.");
+ }
+#else
+ // Default the destructor in release builds, A trivial destructor may enable
+ // better codegen.
+ ~RefCountedBase() = default;
+#endif
+public:
void Retain() const { ++RefCount; }
void Release() const {
@@ -85,10 +102,29 @@
/// A thread-safe version of \c RefCountedBase.
template <class Derived> class ThreadSafeRefCountedBase {
- mutable std::atomic<int> RefCount;
+ mutable std::atomic<int> RefCount{0};
protected:
- ThreadSafeRefCountedBase() : RefCount(0) {}
+ ThreadSafeRefCountedBase() = default;
+ ThreadSafeRefCountedBase(const ThreadSafeRefCountedBase &) {}
+ ThreadSafeRefCountedBase(ThreadSafeRefCountedBase &&) {}
+ ThreadSafeRefCountedBase &operator=(const ThreadSafeRefCountedBase &) {
+ return *this;
+ }
+ ThreadSafeRefCountedBase &operator=(ThreadSafeRefCountedBase &&) {
+ return *this;
+ }
+
+#ifndef NDEBUG
+ ~ThreadSafeRefCountedBase() {
+ assert(RefCount == 0 &&
+ "Destruction occured when there are still references to this.");
+ }
+#else
+ // Default the destructor in release builds, A trivial destructor may enable
+ // better codegen.
+ ~ThreadSafeRefCountedBase() = default;
+#endif
public:
void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -187,9 +187,20 @@
IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher;
};
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This
prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+ static void call(void *Ptr) {
+ static_cast<DynMatcherInterface *>(Ptr)->Release();
+ }
+};
+
} // namespace
-static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
+static llvm::ManagedStatic<TrueMatcherImpl,
+ llvm::object_creator<TrueMatcherImpl>,
+ DynMatcherInterfaceDeleter>
+ TrueMatcherInstance;
bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const {
return getASTContext().getParentMapContext().getTraversalKind() ==
Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===================================================================
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -70,10 +70,27 @@
template <class Derived> class RefCountedBase {
mutable unsigned RefCount = 0;
-public:
+protected:
RefCountedBase() = default;
+ // Copy and move constructors/assignments are no-ops as the RefCount isn't
+ // dictated by the class directly.
RefCountedBase(const RefCountedBase &) {}
+ RefCountedBase(RefCountedBase &&) {}
+ RefCountedBase &operator=(const RefCountedBase &) { return *this; }
+ RefCountedBase &operator=(RefCountedBase &&) { return *this; }
+
+#ifndef NDEBUG
+ ~RefCountedBase() {
+ assert(RefCount == 0 &&
+ "Destruction occured when there are still references to this.");
+ }
+#else
+ // Default the destructor in release builds, A trivial destructor may enable
+ // better codegen.
+ ~RefCountedBase() = default;
+#endif
+public:
void Retain() const { ++RefCount; }
void Release() const {
@@ -85,10 +102,29 @@
/// A thread-safe version of \c RefCountedBase.
template <class Derived> class ThreadSafeRefCountedBase {
- mutable std::atomic<int> RefCount;
+ mutable std::atomic<int> RefCount{0};
protected:
- ThreadSafeRefCountedBase() : RefCount(0) {}
+ ThreadSafeRefCountedBase() = default;
+ ThreadSafeRefCountedBase(const ThreadSafeRefCountedBase &) {}
+ ThreadSafeRefCountedBase(ThreadSafeRefCountedBase &&) {}
+ ThreadSafeRefCountedBase &operator=(const ThreadSafeRefCountedBase &) {
+ return *this;
+ }
+ ThreadSafeRefCountedBase &operator=(ThreadSafeRefCountedBase &&) {
+ return *this;
+ }
+
+#ifndef NDEBUG
+ ~ThreadSafeRefCountedBase() {
+ assert(RefCount == 0 &&
+ "Destruction occured when there are still references to this.");
+ }
+#else
+ // Default the destructor in release builds, A trivial destructor may enable
+ // better codegen.
+ ~ThreadSafeRefCountedBase() = default;
+#endif
public:
void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===================================================================
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -187,9 +187,20 @@
IntrusiveRefCntPtr<DynMatcherInterface> InnerMatcher;
};
+// Use a custom deleter for the TrueMatcherInstance ManagedStatic. This prevents
+// an assert firing when the refcount is nonzero while running its destructor.
+struct DynMatcherInterfaceDeleter {
+ static void call(void *Ptr) {
+ static_cast<DynMatcherInterface *>(Ptr)->Release();
+ }
+};
+
} // namespace
-static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
+static llvm::ManagedStatic<TrueMatcherImpl,
+ llvm::object_creator<TrueMatcherImpl>,
+ DynMatcherInterfaceDeleter>
+ TrueMatcherInstance;
bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const {
return getASTContext().getParentMapContext().getTraversalKind() ==
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
