Boost logo

Boost :

From: Tylo (tylo_at_[hidden])
Date: 2004-08-05 12:15:33


shared_ptr improvement proposal
by T. Lovset

---
note: Parts of this may be long-winded and possible obvious to some of you, 
but I feel it is needed to support the arguments. Let me know what you think.
---
I propose a modification to the smart pointer classes. In my opinion, this 
modification improves the current smart pointer usage in several respects, 
although it is not completely backward compatible.
However, that is also the main purpose of the modification, because as we 
know, the explicit constructors that take a raw pointer is dangerous in 
numerous ways. As an illustration, I show a realistic example that violates 
the smart pointer preconditions in a subtle (and dangerous) manner.
As a background, some quotes from the boost "Proposal to Add General Purpose 
Smart Pointers to the Library Technical Report" 
(http://std.dkuug.dk/jtc1/sc22/wg21/docs/papers/2003/n1450.html):
(1)
> A common concern, dating back to [Colvin94], is that it is very easy to 
violate the preconditions
> of smart pointer operations. This proposal attempts to systematically 
eliminate undefined behavior
> possibilities as long as the user is manipulating already valid shared_ptr 
(and weak_ptr)
> instances, but using the pointer constructor correctly is still up to the 
user.
And: 
> - Widespread existing practice strongly favors the unprotected but powerful 
pointer constructor
>   approach.
(2)
> A. General Principles
> 1. "As Close to Raw Pointers as Possible, but no Closer"
For auto_ptr and shared_ptr, we are familiar with the form:
   shared_ptr<Widget> smartWidgetPtr( new Widget(a, b) );
   
The constructor is made "explicit" in order to avoid implicit and 
unintentional construction of smart pointers, e.g.:
   void f( shared_ptr<Widget> const & wp );
would without explicit constructors accept f(rawWidgetPtr) with disastrous 
results. Still, as mentioned, "explicit" does not make up for all misuses. For 
example, an inexperienced user could try to call f(shared_ptr<Widget>
(rawWidgetPtr)) to make it compile. Fortunately, this is quite easy to spot as 
an error, and therefore "explicit" does it's mission here.
But a much more common and subtle problem may arise when converting from raw 
pointers to smart pointers:
   class WidgetUtil {
   public:
      WidgetUtil(Widget* wp);
   private:
      Widget* m_wp;
   };
   
   WidgetUtil::WidgetUtil(Widget* wp)
   : m_wp(wp) { ... }
Now converting to shared_ptr:
   class WidgetUtil {
   public:
      WidgetUtil(Widget* wp);
   private:
      shared_ptr<Widget> m_wp;
   };
The unmodified constructor still compiles, but the precondition for m_wp is 
violated. One must therefore pay close attention to the constructors to make 
sure that they do not take raw pointer as arguments, which initializes smart 
pointers.
In the case of a big class with a mixture of both smart and raw pointer 
members, the principle of "using the pointer constructor correctly is still up 
to the user", lays an unacceptable heavy burden upon the user (in my opinion) -
 especially when it is possible to avoid this problem.
Before I move on to the proposal, I'll address another drawback with the 
current smart pointer implementations: implicit ctor-initialization and 
assignment to NULL (which is possible with raw pointers) is not supported. 
This would make things much easier when converting back and forth between 
smart and raw pointers. It also breaks the principle of "As Close to Raw 
Pointers as Possible, but no Closer". My proposal makes this possible, and 
thus making the usage closer to that of raw pointers, without introducing any 
side effects.
[As a side note, Joe Guttman pointed out to me that with the proposed nullptr 
and nullptr_t, it will be easy to add these features to the current 
implementations of smart pointers. However, my proposal does not rely on 
nullptr.]
The Proposal
============
Note that this proposal would be beneficial to most smart pointer 
implementation, including shared_ptr and auto_ptr. Unfortunately, auto_ptr is 
already in the standard, so it can't be touched.
I propose this usage (for shared_ptr):
   shared_ptr<Widget> wp1 = NULL;
   shared_ptr<Widget> wp2 = rawptr(new Widget(a, b));
   wp1 = rawptr( new Widget(a, b) );
   wp2 = NULL;
   
Current shared_ptr usage:
   shared_ptr<Widget> wp1( NULL );
   shared_ptr<Widget> wp2( new Widget(a, b) );
   wp1.reset( new Widget(a, b) );
   wp2.reset();
Discussion:
1. As wp1 and wp2 should be raw pointer "look alikes", the proposed usage is 
more intuitive than the current usage, in my opinion, and allows to assign 
NULL without side-effects. 
2. As noted by others [e.g. Andrei Alexandrescu], member functions in smart 
pointers should be avoided if possible. The reset() function makes it hard to 
switch between raw and smart pointers, and also allows mix-ups: wp1.reset(), 
(*wp1).reset(), wp1->reset().
3. The rawptr() function makes initialization and assignment explicit using an 
alternative and better syntax, IMO. E.g., you can verify that every shared_ptr 
owns a valid raw pointer by investigating instances of "rawptr(" in the code. 
Typically, you want to see: rawptr(new SomeClass(..)). To do this is not easy 
with the current implementation. You have to search for:
   a) "shared_ptr<" stack variable constructions of which takes raw pointers 
as arguments.
   b) ".reset(" for re-assigning a new raw pointer. Unfortunately reset() is a 
commonly used name.
   c) shared_ptr's in class member initiation lists, as mentioned in the 
example presented.
   
4. Most importantly, all raw pointers that are assigned to smart pointers must 
go through the rawptr() function (or rawptr_t<T> type), always making 
it "explicit". The problem in the example presented earlier, is therefore 
eliminated.
5. This proposal introduces no additional overhead, as long as the compiler 
really inline the rawptr() function (the operator new call is dominant by an 
order of magnitude in any case).
6. The only obvious problem I see, is the backward compability issue, although 
it still support the "unprotected but powerful pointer constructor approach", 
with a slightly different syntax.
Implementation for shared_ptr
=============================
rawptr.hpp (new file):
---------------------
namespace boost {
  template <class T> class rawptr_t {
      public:
         explicit rawptr_t(T * p) : px(p) {}
         T* get() const { return px; }
      private:
         rawptr_t& operator=(const rawptr_t&);
         T* px;
   };
 
   template <class T> inline rawptr_t<T> rawptr(T * p) {
      return rawptr_t<T>(p);
   }
}
shared_ptr.hpp (diff from boost 1.31.0):
----------------------------------------
***************
*** 109,114 ****
--- 123,129 ----
  
      // Borland 5.5.1 specific workaround
      typedef shared_ptr<T> this_type;
+     struct null_tag {};
  
  public:
  
***************
*** 117,130 ****
      typedef T * pointer;
      typedef typename detail::shared_ptr_traits<T>::reference reference;
  
!     shared_ptr(): px(0), pn() // never throws in 1.30+
      {
      }
  
      template<class Y>
!     explicit shared_ptr(Y * p): px(p), pn(p, checked_deleter<Y>()) // Y must 
be complete
      {
!         detail::sp_enable_shared_from_this(p, p, pn);
      }
  
      //
--- 132,145 ----
      typedef T * pointer;
      typedef typename detail::shared_ptr_traits<T>::reference reference;
  
!     shared_ptr(null_tag* = NULL): px(0), pn() // never throws in 1.30+
      {
      }
  
      template<class Y>
!     shared_ptr(rawptr_t<Y> const & w): px(w.get()), pn(w.get(), 
checked_deleter<Y>()) // Y must be complete
      {
!         detail::sp_enable_shared_from_this(w.get(), w.get(), pn);
      }
  
      //
***************
*** 226,231 ****
--- 241,258 ----
      }
  
  #endif
+     shared_ptr & operator=(null_tag *)
+     {
+         reset();
+         return *this;
+     }
+ 
+ 	template<class Y>
+     shared_ptr & operator=(rawptr_t<Y> const & w)
+     {
+         this_type(w).swap(*this);
+         return *this;
+     }
  
      void reset() // never throws in 1.30+
      {
***************
*** 235,241 ****
      template<class Y> void reset(Y * p) // Y must be complete
      {
          BOOST_ASSERT(p == 0 || p != px); // catch self-reset errors
!         this_type(p).swap(*this);
      }
  
      template<class Y, class D> void reset(Y * p, D d)
--- 262,268 ----
      template<class Y> void reset(Y * p) // Y must be complete
      {
          BOOST_ASSERT(p == 0 || p != px); // catch self-reset errors
!         this_type(rawptr(p)).swap(*this);
      }
  
      template<class Y, class D> void reset(Y * p, D d)
------------------------------------------------------------
Få din egen @start.no-adresse gratis på http://www.start.no/

Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk