[PATCH Boost.smart_ptr] Add intrusive_ptr<>::release()
This provides a way to escape from automatic reference counting, and taking manual control of the reference. Useful when interfacing to a C API that expects a pointer with an elevated reference count. Similar to std::unique_ptr<>::release(). --- include/boost/smart_ptr/intrusive_ptr.hpp | 7 +++++++ intrusive_ptr.html | 23 ++++++++++++++++++----- test/intrusive_ptr_test.cpp | 7 +++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/include/boost/smart_ptr/intrusive_ptr.hpp b/include/boost/smart_ptr/intrusive_ptr.hpp index a855a10..8c73fe2 100644 --- a/include/boost/smart_ptr/intrusive_ptr.hpp +++ b/include/boost/smart_ptr/intrusive_ptr.hpp @@ -151,6 +151,13 @@ public: return px; } + T * release() BOOST_NOEXCEPT + { + T * ret = px; + px = 0; + return ret; + } + T & operator*() const { BOOST_ASSERT( px != 0 ); diff --git a/intrusive_ptr.html b/intrusive_ptr.html index 562d27a..ef6238b 100644 --- a/intrusive_ptr.html +++ b/intrusive_ptr.html @@ -71,6 +71,7 @@ T & <A href="#indirection" >operator*</A>() const; // never throws T * <A href="#indirection" >operator-></A>() const; // never throws T * <A href="#get" >get</A>() const; // never throws + T * <A href="#release" >release</A>(); // never throws operator <A href="#conversions" ><i>unspecified-bool-type</i></A>() const; // never throws @@ -174,11 +175,23 @@ intrusive_ptr & operator=(T * r);</pre> <p><b>Throws:</b> nothing.</p> </blockquote> <h3><a name="get">get</a></h3> - <pre>T * get() const; // never throws</pre> - <blockquote> - <p><b>Returns:</b> the stored pointer.</p> - <p><b>Throws:</b> nothing.</p> - </blockquote> + <pre>T * get() const; // never throws</pre> + <blockquote> + <p><b>Returns:</b> the stored pointer.</p> + <p><b>Throws:</b> nothing.</p> + </blockquote> + <h3><a name="release">release</a></h3> + <pre>T * release(); // never throws</pre> + <blockquote> + <p><b>Returns:</b> the stored pointer.</p> + <p><b>Throws:</b> nothing.</p> + <p><b>Postconditions:</b> <code>get() == 0</code>.</p> + <p><b>Notes:</b> The returned pointer has an elevated reference count. This + allows conversion of an <b>intrusive_ptr</b> back to a raw pointer, + without the performance overhead of acquiring and dropping an extra + reference. It can be viewed as the complement of the + non-reference-incrementing constructor.</p> + </blockquote> <h3><a name="conversions">conversions</a></h3> <pre>operator <i>unspecified-bool-type</i> () const; // never throws</pre> <blockquote> diff --git a/test/intrusive_ptr_test.cpp b/test/intrusive_ptr_test.cpp index 614a9cb..90248fe 100644 --- a/test/intrusive_ptr_test.cpp +++ b/test/intrusive_ptr_test.cpp @@ -330,6 +330,13 @@ void test() BOOST_TEST(get_pointer(px) == px.get()); } + + { + boost::intrusive_ptr<X> px(new X); + X* released = px.release(); + BOOST_TEST(released->use_count() == 1); + delete released; + } } } // namespace n_access -- 1.8.3.1
On Mon, Dec 02, 2013 at 02:57:06PM +0200, Avi Kivity wrote:
This provides a way to escape from automatic reference counting, and taking manual control of the reference. Useful when interfacing to a C API that expects a pointer with an elevated reference count.
Similar to std::unique_ptr<>::release().
Running the risk of bikeshedding, the name could be a bit misleading to some. Examples: CComPtr::Release reduces the refcount. intrusive_ptr_release reduces the refcount. For prior art, CComPtr uses 'Detach' to indicate disowning with retained refcount, and symmetrically 'Attach' to acquiring reponsibility for a pointer without adding refcount. -- Lars Viklund | zao@acc.umu.se
On Monday 02 December 2013 16:37:56 Lars Viklund wrote:
On Mon, Dec 02, 2013 at 02:57:06PM +0200, Avi Kivity wrote:
This provides a way to escape from automatic reference counting, and taking manual control of the reference. Useful when interfacing to a C API that expects a pointer with an elevated reference count.
Similar to std::unique_ptr<>::release().
Running the risk of bikeshedding, the name could be a bit misleading to some.
Examples: CComPtr::Release reduces the refcount. intrusive_ptr_release reduces the refcount.
For prior art, CComPtr uses 'Detach' to indicate disowning with retained refcount, and symmetrically 'Attach' to acquiring reponsibility for a pointer without adding refcount.
I'd vote to follow STL precedent rather than ATL.
On Dec 2, 2013 3:48 PM, "Andrey Semashev" wrote:
I'd vote to follow STL precedent rather than ATL.
But it doesn't do the same thing as unique_ptr::release() void f(std::unique_ptr<int> up) { std::shared_ptr<int> sp(up.release()); // ok } void f(boost::intrusive_ptr<int> ip) { std::shared_ptr<int> sp(ip.release()); // probably not ok }
On Monday 02 December 2013 16:30:52 Jonathan Wakely wrote:
On Dec 2, 2013 3:48 PM, "Andrey Semashev" wrote:
I'd vote to follow STL precedent rather than ATL.
But it doesn't do the same thing as unique_ptr::release()
Yes it does - it resets the pointer without running any deletion logic on the pointed object. Whether it is correct or not in a particular context is another (irrelevant) question. The reset/release pattern is quite established in STL and Boost and I'd prefer not to disturb it. This will only add confusion.
Andrey Semashev wrote:
The reset/release pattern is quite established in STL and Boost and I'd prefer not to disturb it. This will only add confusion.
I agree with Lars and Jonathan, sorry Andrey. Naming the function "release" will add confusion even though you might argue that it will improve consistency.
On Mon, Dec 2, 2013 at 8:45 PM, Peter Dimov
Andrey Semashev wrote:
The reset/release pattern is quite established in STL and Boost and I'd prefer not to disturb it. This will only add confusion.
I agree with Lars and Jonathan, sorry Andrey. Naming the function "release" will add confusion even though you might argue that it will improve consistency.
I'll rename it to detach() if that's the consensus.
On Tue, Dec 3, 2013 at 3:45 AM, Avi Kivity
On Mon, Dec 2, 2013 at 8:45 PM, Peter Dimov
wrote: Andrey Semashev wrote:
The reset/release pattern is quite established in STL and Boost and I'd prefer not to disturb it. This will only add confusion.
I agree with Lars and Jonathan, sorry Andrey. Naming the function "release" will add confusion even though you might argue that it will improve consistency.
I'll rename it to detach() if that's the consensus.
Concern over misuse was a key reason past release() proposals didn't fly(). Changing the name would help, but an explicit caution about misuse in the docs might also help. Thanks for including doc and test patches. That always makes a good impression:-) --Beman
Avi Kivity wrote:
I'll rename it to detach() if that's the consensus.
It so happens that there is already a ticket asking for detach(): https://svn.boost.org/trac/boost/ticket/5690 so you could attach your patch to it. And, as Beman said, thank you for being thorough with it. :-)
On Tue, Dec 3, 2013 at 3:19 PM, Peter Dimov
Avi Kivity wrote:
I'll rename it to detach() if that's the consensus.
It so happens that there is already a ticket asking for detach():
https://svn.boost.org/trac/boost/ticket/5690
so you could attach your patch to it.
Done, hope to see it committed soon.
And, as Beman said, thank you for being thorough with it. :-)
Thanks!
On Tue, Dec 3, 2013 at 11:15 PM, Peter Dimov
Avi Kivity wrote:
Done, hope to see it committed soon.
Well... I'll need to learn git first, because Boost just migrated from SVN to it and things are still a bit murky at the moment. :-)
Try git am -i /path/to/file.patch
On Tue, Dec 3, 2013 at 7:27 PM, Avi Kivity
On Tue, Dec 3, 2013 at 3:19 PM, Peter Dimov
wrote: Avi Kivity wrote:
I'll rename it to detach() if that's the consensus.
It so happens that there is already a ticket asking for detach():
https://svn.boost.org/trac/boost/ticket/5690
so you could attach your patch to it.
Done, hope to see it committed soon.
I see this hasn't been committed yet, is anything missing?
Avi Kivity wrote:
I see this hasn't been committed yet, is anything missing?
There you go: https://github.com/boostorg/smart_ptr/commit/73153d57971849d6cfd13e2bd1bf2b0...
On 12/2/13, Avi Kivity
This provides a way to escape from automatic reference counting, and taking manual control of the reference. Useful when interfacing to a C API that expects a pointer with an elevated reference count.
+ <p><b>Notes:</b> The returned pointer has an elevated reference count. This
I don't think "elevated reference count" is clear enough. How elevated? Elevated more? It should say that it does not decrement the reference count. Or "resets the intrusive_ptr without decrementing the reference count". I don't think either name (release/detach) is clear enough. I'd prefer something like detach_retaining_count() which isn't quite right either, but I'd be happy with a name that was long, *ugly*, and clear. - Ugly because it is a dangerous function. Tony
On Wednesday 04 December 2013 11:30:47 Gottlob Frege wrote:
On 12/2/13, Avi Kivity
wrote: This provides a way to escape from automatic reference counting, and taking manual control of the reference. Useful when interfacing to a C API that expects a pointer with an elevated reference count.
+ <p><b>Notes:</b> The returned pointer has an elevated reference count. This
I don't think "elevated reference count" is clear enough. How elevated? Elevated more?
It should say that it does not decrement the reference count. Or "resets the intrusive_ptr without decrementing the reference count".
...without calling intrusive_ptr_release(), since technically intrusive_ptr itself doesn't operate on counter.
I don't think either name (release/detach) is clear enough. I'd prefer something like
detach_retaining_count()
which isn't quite right either, but I'd be happy with a name that was long, *ugly*, and clear. - Ugly because it is a dangerous function.
I don't think it is any more dangerous than unique_ptr::release(), so why should it be ugly? Next to release(), detach() is my second preference.
participants (7)
-
Andrey Semashev
-
Avi Kivity
-
Beman Dawes
-
Gottlob Frege
-
Jonathan Wakely
-
Lars Viklund
-
Peter Dimov