From 517714ca17ec9f2583cf6649ab4eb4a186331f5f Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Thu, 29 Dec 2016 19:59:26 +0000 Subject: [PATCH] [ADT] Delete RefCountedBaseVPTR. Summary: This class is unnecessary. Its comment indicated that it was a compile error to allocate an instance of a class that inherits from RefCountedBaseVPTR on the stack. This may have been true at one point, but it's not today. Moreover you really do not want to allocate *any* refcounted object on the stack, vptrs or not, so if we did have a way to prevent these objects from being stack-allocated, we'd want to apply it to regular RefCountedBase too, obviating the need for a separate RefCountedBaseVPTR class. It seems that the main way RefCountedBaseVPTR provides safety is by making its subclass's destructor virtual. This may have been helpful at one point, but these days clang will emit an error if you define a class with virtual functions that inherits from RefCountedBase but doesn't have a virtual destructor. Reviewers: compnerd, dblaikie Subscribers: cfe-commits, klimek, llvm-commits, mgorny Differential Revision: https://reviews.llvm.org/D28162 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@290717 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/ADT/IntrusiveRefCntPtr.h | 43 ++++-------------------- lib/Support/CMakeLists.txt | 1 - lib/Support/IntrusiveRefCntPtr.cpp | 14 -------- unittests/ADT/IntrusiveRefCntPtrTest.cpp | 37 ++++++++++---------- 4 files changed, 24 insertions(+), 71 deletions(-) delete mode 100644 lib/Support/IntrusiveRefCntPtr.cpp diff --git a/include/llvm/ADT/IntrusiveRefCntPtr.h b/include/llvm/ADT/IntrusiveRefCntPtr.h index b0c2ce76a33..287b40f09df 100644 --- a/include/llvm/ADT/IntrusiveRefCntPtr.h +++ b/include/llvm/ADT/IntrusiveRefCntPtr.h @@ -9,9 +9,9 @@ // // This file defines IntrusiveRefCntPtr, a template class that // implements a "smart" pointer for objects that maintain their own -// internal reference count, and RefCountedBase/RefCountedBaseVPTR, two -// generic base classes for objects that wish to have their lifetimes -// managed using reference counting. +// internal reference count, and RefCountedBase, a generic base class +// for objects that wish to have their lifetimes managed using reference +// counting. // // IntrusiveRefCntPtr is similar to Boost's intrusive_ptr with added // LLVM-style casting. @@ -52,36 +52,6 @@ namespace llvm { } }; -//===----------------------------------------------------------------------===// -/// RefCountedBaseVPTR - A class that has the same function as -/// RefCountedBase, but with a virtual destructor. Should be used -/// instead of RefCountedBase for classes that already have virtual -/// methods to enforce dynamic allocation via 'new'. Classes that -/// inherit from RefCountedBaseVPTR can't be allocated on stack - -/// attempting to do this will produce a compile error. -//===----------------------------------------------------------------------===// - class RefCountedBaseVPTR { - mutable unsigned ref_cnt = 0; - - virtual void anchor(); - - protected: - RefCountedBaseVPTR() = default; - RefCountedBaseVPTR(const RefCountedBaseVPTR &) : ref_cnt(0) {} - - virtual ~RefCountedBaseVPTR() = default; - - void Retain() const { ++ref_cnt; } - void Release() const { - assert (ref_cnt > 0 && "Reference count is already zero."); - if (--ref_cnt == 0) delete this; - } - - template - friend struct IntrusiveRefCntPtrInfo; - }; - - template struct IntrusiveRefCntPtrInfo { static void retain(T *obj) { obj->Retain(); } static void release(T *obj) { obj->Release(); } @@ -124,10 +94,9 @@ public: /// wrapping NULL pointers. /// /// Reference counting is implemented via calls to -/// Obj->Retain()/Obj->Release(). Release() is required to destroy -/// the object when the reference count reaches zero. Inheriting from -/// RefCountedBase/RefCountedBaseVPTR takes care of this -/// automatically. +/// Obj->Retain()/Obj->Release(). Release() is required to destroy the +/// object when the reference count reaches zero. Inheriting from +/// RefCountedBase takes care of this automatically. //===----------------------------------------------------------------------===// template class IntrusiveRefCntPtr { diff --git a/lib/Support/CMakeLists.txt b/lib/Support/CMakeLists.txt index 03addcbcd16..ca344b1dc05 100644 --- a/lib/Support/CMakeLists.txt +++ b/lib/Support/CMakeLists.txt @@ -61,7 +61,6 @@ add_llvm_library(LLVMSupport Hashing.cpp IntEqClasses.cpp IntervalMap.cpp - IntrusiveRefCntPtr.cpp JamCRC.cpp LEB128.cpp LineIterator.cpp diff --git a/lib/Support/IntrusiveRefCntPtr.cpp b/lib/Support/IntrusiveRefCntPtr.cpp deleted file mode 100644 index a8b45593ae7..00000000000 --- a/lib/Support/IntrusiveRefCntPtr.cpp +++ /dev/null @@ -1,14 +0,0 @@ -//== IntrusiveRefCntPtr.cpp - Smart Refcounting Pointer ----------*- C++ -*-==// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// - -#include "llvm/ADT/IntrusiveRefCntPtr.h" - -using namespace llvm; - -void RefCountedBaseVPTR::anchor() { } diff --git a/unittests/ADT/IntrusiveRefCntPtrTest.cpp b/unittests/ADT/IntrusiveRefCntPtrTest.cpp index c67ec130912..143a8cc4910 100644 --- a/unittests/ADT/IntrusiveRefCntPtrTest.cpp +++ b/unittests/ADT/IntrusiveRefCntPtrTest.cpp @@ -10,30 +10,29 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "gtest/gtest.h" -namespace { -struct VirtualRefCounted : public llvm::RefCountedBaseVPTR { - virtual void f() {} -}; -} - namespace llvm { -// Run this test with valgrind to detect memory leaks. -TEST(IntrusiveRefCntPtr, RefCountedBaseVPTRCopyDoesNotLeak) { - VirtualRefCounted *V1 = new VirtualRefCounted; - IntrusiveRefCntPtr R1 = V1; - VirtualRefCounted *V2 = new VirtualRefCounted(*V1); - IntrusiveRefCntPtr R2 = V2; -} +namespace { +struct SimpleRefCounted : public RefCountedBase { + SimpleRefCounted() { ++NumInstances; } + SimpleRefCounted(const SimpleRefCounted &) { ++NumInstances; } + ~SimpleRefCounted() { --NumInstances; } -struct SimpleRefCounted : public RefCountedBase {}; + static int NumInstances; +}; +int SimpleRefCounted::NumInstances = 0; +} // anonymous namespace -// Run this test with valgrind to detect memory leaks. TEST(IntrusiveRefCntPtr, RefCountedBaseCopyDoesNotLeak) { - SimpleRefCounted *S1 = new SimpleRefCounted; - IntrusiveRefCntPtr R1 = S1; - SimpleRefCounted *S2 = new SimpleRefCounted(*S1); - IntrusiveRefCntPtr R2 = S2; + EXPECT_EQ(0, SimpleRefCounted::NumInstances); + { + SimpleRefCounted *S1 = new SimpleRefCounted; + IntrusiveRefCntPtr R1 = S1; + SimpleRefCounted *S2 = new SimpleRefCounted(*S1); + IntrusiveRefCntPtr R2 = S2; + EXPECT_EQ(2, SimpleRefCounted::NumInstances); + } + EXPECT_EQ(0, SimpleRefCounted::NumInstances); } struct InterceptRefCounted : public RefCountedBase { -- 2.49.0