Skip to content

Commit dc361d5

Browse files
committed
[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
1 parent 0a39106 commit dc361d5

File tree

2 files changed

+30
-10
lines changed

2 files changed

+30
-10
lines changed

clang/lib/ASTMatchers/ASTMatchersInternal.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,15 +150,9 @@ class IdDynMatcher : public DynMatcherInterface {
150150
};
151151

152152
/// A matcher that always returns true.
153-
///
154-
/// We only ever need one instance of this matcher, so we create a global one
155-
/// and reuse it to reduce the overhead of the matcher and increase the chance
156-
/// of cache hits.
157153
class TrueMatcherImpl : public DynMatcherInterface {
158154
public:
159-
TrueMatcherImpl() {
160-
Retain(); // Reference count will never become zero.
161-
}
155+
TrueMatcherImpl() = default;
162156

163157
bool dynMatches(const DynTypedNode &, ASTMatchFinder *,
164158
BoundNodesTreeBuilder *) const override {
@@ -193,8 +187,6 @@ class DynTraversalMatcherImpl : public DynMatcherInterface {
193187

194188
} // namespace
195189

196-
static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
197-
198190
bool ASTMatchFinder::isTraversalIgnoringImplicitNodes() const {
199191
return getASTContext().getParentMapContext().getTraversalKind() ==
200192
TK_IgnoreUnlessSpelledInSource;
@@ -273,7 +265,12 @@ DynTypedMatcher::withTraversalKind(ast_type_traits::TraversalKind TK) {
273265
}
274266

275267
DynTypedMatcher DynTypedMatcher::trueMatcher(ASTNodeKind NodeKind) {
276-
return DynTypedMatcher(NodeKind, NodeKind, &*TrueMatcherInstance);
268+
// We only ever need one instance of TrueMatcherImpl, so we create a static
269+
// instance and reuse it to reduce the overhead of the matcher and increase
270+
// the chance of cache hits.
271+
static const llvm::IntrusiveRefCntPtr<TrueMatcherImpl> Instance =
272+
new TrueMatcherImpl();
273+
return DynTypedMatcher(NodeKind, NodeKind, Instance);
277274
}
278275

279276
bool DynTypedMatcher::canMatchNodesOfKind(ASTNodeKind Kind) const {

llvm/include/llvm/ADT/IntrusiveRefCntPtr.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,18 @@ template <class Derived> class RefCountedBase {
7575
RefCountedBase(const RefCountedBase &) {}
7676
RefCountedBase &operator=(const RefCountedBase &) = delete;
7777

78+
#ifndef NDEBUG
79+
~RefCountedBase() {
80+
assert(RefCount == 0 &&
81+
"Destruction occured when there are still references to this.");
82+
}
83+
#else
84+
// Default the destructor in release builds, A trivial destructor may enable
85+
// better codegen.
86+
~RefCountedBase() = default;
87+
#endif
88+
89+
public:
7890
void Retain() const { ++RefCount; }
7991

8092
void Release() const {
@@ -94,6 +106,17 @@ template <class Derived> class ThreadSafeRefCountedBase {
94106
ThreadSafeRefCountedBase &
95107
operator=(const ThreadSafeRefCountedBase &) = delete;
96108

109+
#ifndef NDEBUG
110+
~ThreadSafeRefCountedBase() {
111+
assert(RefCount == 0 &&
112+
"Destruction occured when there are still references to this.");
113+
}
114+
#else
115+
// Default the destructor in release builds, A trivial destructor may enable
116+
// better codegen.
117+
~ThreadSafeRefCountedBase() = default;
118+
#endif
119+
97120
public:
98121
void Retain() const { RefCount.fetch_add(1, std::memory_order_relaxed); }
99122

0 commit comments

Comments
 (0)