<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 2, 2017 at 11:51 AM Justin Lebar <<a href="mailto:jlebar@google.com">jlebar@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, it is safe to allocate an instance of an object which inherits<br class="gmail_msg">
from RefCountedBase on the stack (or whatever) so long as you never<br class="gmail_msg">
call retain().<br class="gmail_msg">
<br class="gmail_msg">
However, this is a difficult invariant to enforce, because of its<br class="gmail_msg">
global nature.</blockquote><div><br></div><div>Not sure that it's so hard to enforce - usually you pick an ownership model for an object and stick with it. Certainly any places where such a value is treated as a value type (ie: not allocated with new directly) it'd be fine - in the same way that if I create a std::string on the stack it's unlikely I'll accidentally takes its address and put that address in a std::unique_ptr, or the like.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Therefore I think it's probably good advice to say<br class="gmail_msg">
that such objects "should always be instantiated on the heap".<br class="gmail_msg">
<br class="gmail_msg">
Note that this is prescriptive, not descriptive -- it does not say<br class="gmail_msg">
that it's always unsafe to allocate such objects on the stack. I<br class="gmail_msg">
agree that would be false.<br class="gmail_msg"></blockquote><div><br></div><div>I think that may be an interesting distinction in formal english, but probably isn't how the average reader might read the comment. "should always" sounds fairly prescriptive (always). I think I'd certainly read "should be" and "should always" as "that's the only way to use this correctly". "should usually" is the wording I would expect if it was "this is the right thing most of the time but there are other ways to use it".<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
On Mon, Jan 2, 2017 at 11:45 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> On Thu, Dec 29, 2016 at 12:10 PM Justin Lebar via llvm-commits<br class="gmail_msg">
> <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
>><br class="gmail_msg">
>> Author: jlebar<br class="gmail_msg">
>> Date: Thu Dec 29 13:59:38 2016<br class="gmail_msg">
>> New Revision: 290720<br class="gmail_msg">
>><br class="gmail_msg">
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=290720&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=290720&view=rev</a><br class="gmail_msg">
>> Log:<br class="gmail_msg">
>> [ADT] Rewrite IntrusiveRefCntPtr's comments. NFC<br class="gmail_msg">
>><br class="gmail_msg">
>> Edit for voice, and also add examples. In particular, add an<br class="gmail_msg">
>> explanation for why you might want to specialize IntrusiveRefCntPtrInfo,<br class="gmail_msg">
>> which is not obvious.<br class="gmail_msg">
>><br class="gmail_msg">
>> Modified:<br class="gmail_msg">
>> llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h<br class="gmail_msg">
>><br class="gmail_msg">
>> Modified: llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h<br class="gmail_msg">
>> URL:<br class="gmail_msg">
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h?rev=290720&r1=290719&r2=290720&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h?rev=290720&r1=290719&r2=290720&view=diff</a><br class="gmail_msg">
>><br class="gmail_msg">
>> ==============================================================================<br class="gmail_msg">
>> --- llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h (original)<br class="gmail_msg">
>> +++ llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h Thu Dec 29 13:59:38<br class="gmail_msg">
>> 2016<br class="gmail_msg">
>> @@ -7,14 +7,49 @@<br class="gmail_msg">
>> //<br class="gmail_msg">
>><br class="gmail_msg">
>> //===----------------------------------------------------------------------===//<br class="gmail_msg">
>> //<br class="gmail_msg">
>> -// This file defines IntrusiveRefCntPtr, a template class that<br class="gmail_msg">
>> -// implements a "smart" pointer for objects that maintain their own<br class="gmail_msg">
>> -// internal reference count, and RefCountedBase, a generic base class<br class="gmail_msg">
>> -// for objects that wish to have their lifetimes managed using reference<br class="gmail_msg">
>> -// counting.<br class="gmail_msg">
>> +// This file defines the RefCountedBase, ThreadSafeRefCountedBase, and<br class="gmail_msg">
>> +// IntrusiveRefCntPtr classes.<br class="gmail_msg">
>> //<br class="gmail_msg">
>> -// IntrusiveRefCntPtr is similar to Boost's intrusive_ptr with added<br class="gmail_msg">
>> -// LLVM-style casting.<br class="gmail_msg">
>> +// IntrusiveRefCntPtr is a smart pointer to an object which maintains a<br class="gmail_msg">
>> +// reference count. (ThreadSafe)RefCountedBase is a mixin class that<br class="gmail_msg">
>> adds a<br class="gmail_msg">
>> +// refcount member variable and methods for updating the refcount. An<br class="gmail_msg">
>> object<br class="gmail_msg">
>> +// that inherits from (ThreadSafe)RefCountedBase deletes itself when its<br class="gmail_msg">
>> +// refcount hits zero.<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// For example:<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// class MyClass : public RefCountedBase<MyClass> {};<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// void foo() {<br class="gmail_msg">
>> +// // Objects that inherit from RefCountedBase should always be<br class="gmail_msg">
>> instantiated<br class="gmail_msg">
>> +// // on the heap, never on the stack.<br class="gmail_msg">
>> +// IntrusiveRefCntPtr<MyClass> Ptr1(new MyClass());<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// // Copying an IntrusiveRefCntPtr increases the pointee's refcount<br class="gmail_msg">
>> by 1.<br class="gmail_msg">
>> +// IntrusiveRefCntPtr<MyClass> Ptr2(Ptr1);<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// // Constructing an IntrusiveRefCntPtr has no effect on the<br class="gmail_msg">
>> object's<br class="gmail_msg">
>> +// // refcount. After a move, the moved-from pointer is null.<br class="gmail_msg">
>> +// IntrusiveRefCntPtr<MyClass> Ptr3(std::move(Ptr1));<br class="gmail_msg">
>> +// assert(Ptr1 == nullptr);<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// // Clearing an IntrusiveRefCntPtr decreases the pointee's refcount<br class="gmail_msg">
>> by 1.<br class="gmail_msg">
>> +// Ptr2.reset();<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// // The object deletes itself when we return from the function,<br class="gmail_msg">
>> because<br class="gmail_msg">
>> +// // Ptr3's destructor decrements its refcount to 0.<br class="gmail_msg">
>> +// }<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// You can use IntrusiveRefCntPtr with isa<T>(), dyn_cast<T>(), etc.:<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// IntrusiveRefCntPtr<MyClass> Ptr(new MyClass());<br class="gmail_msg">
>> +// OtherClass *Other = dyn_cast<OtherClass>(Ptr); // Ptr.get() not<br class="gmail_msg">
>> required<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// IntrusiveRefCntPtr works with any class that<br class="gmail_msg">
>> +//<br class="gmail_msg">
>> +// - inherits from (ThreadSafe)RefCountedBase,<br class="gmail_msg">
>> +// - has Retain() and Release() methods, or<br class="gmail_msg">
>> +// - specializes IntrusiveRefCntPtrInfo.<br class="gmail_msg">
>> //<br class="gmail_msg">
>><br class="gmail_msg">
>> //===----------------------------------------------------------------------===//<br class="gmail_msg">
>><br class="gmail_msg">
>> @@ -27,16 +62,15 @@<br class="gmail_msg">
>><br class="gmail_msg">
>> namespace llvm {<br class="gmail_msg">
>><br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> -/// RefCountedBase - A generic base class for objects that wish to<br class="gmail_msg">
>> -/// have their lifetimes managed using reference counts. Classes<br class="gmail_msg">
>> -/// subclass RefCountedBase to obtain such functionality, and are<br class="gmail_msg">
>> -/// typically handled with IntrusiveRefCntPtr "smart pointers" (see<br class="gmail_msg">
>> below)<br class="gmail_msg">
>> -/// which automatically handle the management of reference counts.<br class="gmail_msg">
>> -/// Objects that subclass RefCountedBase should not be allocated on<br class="gmail_msg">
>> -/// the stack, as invoking "delete" (which is called when the<br class="gmail_msg">
>> -/// reference count hits 0) on such objects is an error.<br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> +/// A CRTP mixin class that adds reference counting to a type.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// The lifetime of an object which inherits from RefCountedBase is<br class="gmail_msg">
>> managed by<br class="gmail_msg">
>> +/// calls to Release() and Retain(), which increment and decrement the<br class="gmail_msg">
>> object's<br class="gmail_msg">
>> +/// refcount, respectively. When a Release() call decrements the<br class="gmail_msg">
>> refcount to 0,<br class="gmail_msg">
>> +/// the object deletes itself.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// Objects that inherit from RefCountedBase should always be allocated<br class="gmail_msg">
>> with<br class="gmail_msg">
>> +/// operator new.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> Is this accurate? It looks like one could use a RefCountedBase without any<br class="gmail_msg">
> release/retain calls - with any external lifetime management (stack,<br class="gmail_msg">
> unique_ptr, container, etc).<br class="gmail_msg">
><br class="gmail_msg">
>><br class="gmail_msg">
>> template <class Derived> class RefCountedBase {<br class="gmail_msg">
>> mutable unsigned RefCount = 0;<br class="gmail_msg">
>><br class="gmail_msg">
>> @@ -52,18 +86,7 @@ public:<br class="gmail_msg">
>> }<br class="gmail_msg">
>> };<br class="gmail_msg">
>><br class="gmail_msg">
>> -template <typename T> struct IntrusiveRefCntPtrInfo {<br class="gmail_msg">
>> - static void retain(T *obj) { obj->Retain(); }<br class="gmail_msg">
>> - static void release(T *obj) { obj->Release(); }<br class="gmail_msg">
>> -};<br class="gmail_msg">
>> -<br class="gmail_msg">
>> -/// \brief A thread-safe version of \c llvm::RefCountedBase.<br class="gmail_msg">
>> -///<br class="gmail_msg">
>> -/// A generic base class for objects that wish to have their lifetimes<br class="gmail_msg">
>> managed<br class="gmail_msg">
>> -/// using reference counts. Classes subclass \c ThreadSafeRefCountedBase<br class="gmail_msg">
>> to<br class="gmail_msg">
>> -/// obtain such functionality, and are typically handled with<br class="gmail_msg">
>> -/// \c IntrusiveRefCntPtr "smart pointers" which automatically handle the<br class="gmail_msg">
>> -/// management of reference counts.<br class="gmail_msg">
>> +/// A thread-safe version of \c RefCountedBase.<br class="gmail_msg">
>> template <class Derived> class ThreadSafeRefCountedBase {<br class="gmail_msg">
>> mutable std::atomic<int> RefCount;<br class="gmail_msg">
>><br class="gmail_msg">
>> @@ -81,22 +104,37 @@ public:<br class="gmail_msg">
>> }<br class="gmail_msg">
>> };<br class="gmail_msg">
>><br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> -/// IntrusiveRefCntPtr - A template class that implements a "smart<br class="gmail_msg">
>> pointer"<br class="gmail_msg">
>> -/// that assumes the wrapped object has a reference count associated<br class="gmail_msg">
>> -/// with it that can be managed via calls to<br class="gmail_msg">
>> -/// IntrusivePtrAddRef/IntrusivePtrRelease. The smart pointers<br class="gmail_msg">
>> -/// manage reference counts via the RAII idiom: upon creation of<br class="gmail_msg">
>> -/// smart pointer the reference count of the wrapped object is<br class="gmail_msg">
>> -/// incremented and upon destruction of the smart pointer the<br class="gmail_msg">
>> -/// reference count is decremented. This class also safely handles<br class="gmail_msg">
>> -/// wrapping NULL pointers.<br class="gmail_msg">
>> -///<br class="gmail_msg">
>> -/// Reference counting is implemented via calls to<br class="gmail_msg">
>> -/// Obj->Retain()/Obj->Release(). Release() is required to destroy the<br class="gmail_msg">
>> -/// object when the reference count reaches zero. Inheriting from<br class="gmail_msg">
>> -/// RefCountedBase takes care of this automatically.<br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> +/// Class you can specialize to provide custom retain/release<br class="gmail_msg">
>> functionality for<br class="gmail_msg">
>> +/// a type.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// Usually specializing this class is not necessary, as<br class="gmail_msg">
>> IntrusiveRefCntPtr<br class="gmail_msg">
>> +/// works with any type which defines Retain() and Release() functions --<br class="gmail_msg">
>> you<br class="gmail_msg">
>> +/// can define those functions yourself if RefCountedBase doesn't work<br class="gmail_msg">
>> for you.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// One case when you might want to specialize this type is if you have<br class="gmail_msg">
>> +/// - Foo.h defines type Foo and includes Bar.h, and<br class="gmail_msg">
>> +/// - Bar.h uses IntrusiveRefCntPtr<Foo> in inline functions.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// Because Foo.h includes Bar.h, Bar.h can't include Foo.h in order to<br class="gmail_msg">
>> pull in<br class="gmail_msg">
>> +/// the declaration of Foo. Without the declaration of Foo, normally<br class="gmail_msg">
>> Bar.h<br class="gmail_msg">
>> +/// wouldn't be able to use IntrusiveRefCntPtr<Foo>, which wants to call<br class="gmail_msg">
>> +/// T::Retain and T::Release.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// To resolve this, Bar.h could include a third header, FooFwd.h, which<br class="gmail_msg">
>> +/// forward-declares Foo and specializes IntrusiveRefCntPtrInfo<Foo>.<br class="gmail_msg">
>> Then<br class="gmail_msg">
>> +/// Bar.h could use IntrusiveRefCntPtr<Foo>, although it still couldn't<br class="gmail_msg">
>> call any<br class="gmail_msg">
>> +/// functions on Foo itself, because Foo would be an incomplete type.<br class="gmail_msg">
>> +template <typename T> struct IntrusiveRefCntPtrInfo {<br class="gmail_msg">
>> + static void retain(T *obj) { obj->Retain(); }<br class="gmail_msg">
>> + static void release(T *obj) { obj->Release(); }<br class="gmail_msg">
>> +};<br class="gmail_msg">
>> +<br class="gmail_msg">
>> +/// A smart pointer to a reference-counted object that inherits from<br class="gmail_msg">
>> +/// RefCountedBase or ThreadSafeRefCountedBase.<br class="gmail_msg">
>> +///<br class="gmail_msg">
>> +/// This class increments its pointee's reference count when it is<br class="gmail_msg">
>> created, and<br class="gmail_msg">
>> +/// decrements its refcount when it's destroyed (or is changed to point<br class="gmail_msg">
>> to a<br class="gmail_msg">
>> +/// different object).<br class="gmail_msg">
>> template <typename T> class IntrusiveRefCntPtr {<br class="gmail_msg">
>> T *Obj = nullptr;<br class="gmail_msg">
>><br class="gmail_msg">
>> @@ -208,10 +246,8 @@ bool operator!=(const IntrusiveRefCntPtr<br class="gmail_msg">
>> return !(A == B);<br class="gmail_msg">
>> }<br class="gmail_msg">
>><br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> -// LLVM-style downcasting support for IntrusiveRefCntPtr objects<br class="gmail_msg">
>><br class="gmail_msg">
>> -//===----------------------------------------------------------------------===//<br class="gmail_msg">
>> -<br class="gmail_msg">
>> +// Make IntrusiveRefCntPtr work with dyn_cast, isa, and the other idioms<br class="gmail_msg">
>> from<br class="gmail_msg">
>> +// Casting.h.<br class="gmail_msg">
>> template <typename From> struct simplify_type;<br class="gmail_msg">
>><br class="gmail_msg">
>> template <class T> struct simplify_type<IntrusiveRefCntPtr<T>> {<br class="gmail_msg">
>><br class="gmail_msg">
>><br class="gmail_msg">
>> _______________________________________________<br class="gmail_msg">
>> llvm-commits mailing list<br class="gmail_msg">
>> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
</blockquote></div></div>