[PATCH] D92480: [llvm] Unify interface of (ThreadSafe)?RefCountedBase

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 309489.
njames93 added a comment.

Made this just focus on the destruction asserts.


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,11 +70,23 @@
 template <class Derived> class RefCountedBase {
   mutable unsigned RefCount = 0;
 
-public:
+protected:
   RefCountedBase() = default;
   RefCountedBase(const RefCountedBase &) {}
   RefCountedBase &operator=(const RefCountedBase &) = delete;
 
+#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 {
@@ -94,6 +106,17 @@
   ThreadSafeRefCountedBase &
   operator=(const ThreadSafeRefCountedBase &) = delete;
 
+#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
@@ -191,9 +191,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() ==


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92480.309489.patch
Type: text/x-patch
Size: 2394 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201204/13b14302/attachment.bin>