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

[Nathan James via Phabricator via llvm-commits]

njames93 created this revision.
njames93 added a reviewer: benlangmuir.
Herald added subscribers: llvm-commits, dexonsmith, jfb.
Herald added a project: LLVM.
njames93 requested review of this revision.

Implement all special member functions for these 2 classes, with copy/move operations being no-ops as the RefCount is related to the physical object in memory, not its state.
Added a trivial destructor in release mode and in debug mode a destructor that asserts RefCount is indeed zero.
This ensure people aren't manually (maybe accidentally) destroying these objects like in this contrived example.

  {
    std::unique_ptr<SomethingRefCounted> Object;
    holdIntrusiveOwnership(Object.get());
    // Object Destructor called here will assert.
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92480

Files:
  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); }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92480.308949.patch
Type: text/x-patch
Size: 2125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201202/407442fc/attachment.bin>