njames93 updated this revision to Diff 309120. njames93 added a comment. Added test cases demonstrating copy and move behaviour.
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 llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp
Index: llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp =================================================================== --- llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp +++ llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp @@ -63,4 +63,54 @@ EXPECT_TRUE(Retained); } +namespace { +class DestructionCounter : public RefCountedBase<DestructionCounter> { +public: + DestructionCounter() {} + DestructionCounter(const DestructionCounter &) = default; + DestructionCounter(DestructionCounter &&) = default; + DestructionCounter &operator=(const DestructionCounter &) = default; + DestructionCounter &operator=(DestructionCounter &&) = default; + + ~DestructionCounter() { ++Count; } + + static unsigned getCount() { return Count; } + static void resetCount() { Count = 0; } + +private: + static unsigned Count; +}; +unsigned DestructionCounter::Count = 0U; +} // namespace + +TEST(IntrusiveRefCntPtr, CopyMove) { + { + + IntrusiveRefCntPtr<DestructionCounter> FirstRef = new DestructionCounter(); + { + IntrusiveRefCntPtr<DestructionCounter> SecondRef = FirstRef; + // FirstRef and SecondRef both hold the same object, which has a ref count + // of 2. + + IntrusiveRefCntPtr<DestructionCounter> Copy = + new DestructionCounter(*FirstRef); + // Copy holds an object which is a copy of the object stored in FirstRef, + // but its ref count is only 1. + + IntrusiveRefCntPtr<DestructionCounter> Move = + new DestructionCounter(std::move(*FirstRef)); + // This follows the same logic as Copy above so its ref count is 1 as + // well. + } + // Copy and Move should destroy their underlying object in the destructor as + // the ref count should drop to 0. SecondRef destructor should drop its ref + // count to 1, therefore keeping the underlying object alive. + EXPECT_EQ(DestructionCounter::getCount(), 2U); + DestructionCounter::resetCount(); + } + // Now that FirstRef destructor has ran, the ref count should drop to 0 and + // destroy the underlying object + EXPECT_EQ(DestructionCounter::getCount(), 1U); +} + } // end namespace llvm 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