Author: Nathan James Date: 2020-12-07T20:20:08Z New Revision: dc361d5c2a2dd34839ff4cd48844cf1bf9a87c62
URL: https://github.com/llvm/llvm-project/commit/dc361d5c2a2dd34839ff4cd48844cf1bf9a87c62 DIFF: https://github.com/llvm/llvm-project/commit/dc361d5c2a2dd34839ff4cd48844cf1bf9a87c62.diff LOG: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors 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. ```lang=c++ { std::unique_ptr<SomethingRefCounted> Object; holdIntrusiveOwnership(Object.get()); // Object Destructor called here will assert. } ``` Reviewed By: dblaikie Differential Revision: https://reviews.llvm.org/D92480 Added: Modified: clang/lib/ASTMatchers/ASTMatchersInternal.cpp llvm/include/llvm/ADT/IntrusiveRefCntPtr.h Removed: ################################################################################ diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp index 257a893ccaa6..7f96f4bc03af 100644 --- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -150,15 +150,9 @@ class IdDynMatcher : public DynMatcherInterface { }; /// A matcher that always returns true. -/// -/// We only ever need one instance of this matcher, so we create a global one -/// and reuse it to reduce the overhead of the matcher and increase the chance -/// of cache hits. class TrueMatcherImpl : public DynMatcherInterface { public: - TrueMatcherImpl() { - Retain(); // Reference count will never become zero. - } + TrueMatcherImpl() = default; bool dynMatches(const DynTypedNode &, ASTMatchFinder *, BoundNodesTreeBuilder *) const override { @@ -193,8 +187,6 @@ class DynTraversalMatcherImpl : public DynMatcherInterface { } // namespace -static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance; - bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const { return getASTContext().getParentMapContext().getTraversalKind() == TK_IgnoreUnlessSpelledInSource; @@ -273,7 +265,12 @@ DynTypedMatcher::withTraversalKind(ast_type_traits::TraversalKind TK) { } DynTypedMatcher DynTypedMatcher::trueMatcher(ASTNodeKind NodeKind) { - return DynTypedMatcher(NodeKind, NodeKind, &*TrueMatcherInstance); + // We only ever need one instance of TrueMatcherImpl, so we create a static + // instance and reuse it to reduce the overhead of the matcher and increase + // the chance of cache hits. + static const llvm::IntrusiveRefCntPtr<TrueMatcherImpl> Instance = + new TrueMatcherImpl(); + return DynTypedMatcher(NodeKind, NodeKind, Instance); } bool DynTypedMatcher::canMatchNodesOfKind(ASTNodeKind Kind) const { diff --git a/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h b/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h index 41e5e6b9f8ed..7e6585378ec4 100644 --- a/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h +++ b/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h @@ -75,6 +75,18 @@ template <class Derived> class RefCountedBase { 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 @@ template <class Derived> class ThreadSafeRefCountedBase { 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); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits