Better support for rvalue from-python conversions of shared_ptr:

always return a pointer that holds the owning python object *unless*
the python object contains a NULL shared_ptr holder of the right type.


[SVN r28947]

include/boost/python/converter/registered.hpp

@@ -10,8 +10,16 @@
 # include <boost/type_traits/transform_traits.hpp>
 # include <boost/type_traits/cv_traits.hpp>
 # include <boost/detail/workaround.hpp>
+# include <boost/type.hpp>
 
-namespace boost { namespace python { namespace converter { 
+namespace boost {
+
+// You'll see shared_ptr mentioned in this header because we need to
+// note which types are shared_ptrs in their registrations, to
+// implement special shared_ptr handling for rvalue conversions.
+template <class T> class shared_ptr;
+
+namespace python { namespace converter { 
 
 struct registration;
 
@@ -26,9 +34,9 @@ namespace detail
 
 template <class T>
 struct registered
-    : detail::registered_base<
+  : detail::registered_base<
         typename add_reference<
-           typename add_cv<T>::type
+            typename add_cv<T>::type
         >::type
     >
 {
@@ -50,10 +58,52 @@ struct registered<T&>
 //
 namespace detail
 {
+  inline void
+  register_shared_ptr(...)
+  {
+  }
+  
+  template <class T>
+  inline void
+  register_shared_ptr(type<shared_ptr<T> >)
+  {
+      registry::lookup_shared_ptr(type_id<shared_ptr<T> >());
+  }
+  
+  template <class T>
+  inline void
+  register_shared_ptr(type<shared_ptr<T> const>)
+  {
+      detail::register_shared_ptr(type<shared_ptr<T> >());
+  }
+  
+  template <class T>
+  inline void
+  register_shared_ptr(type<shared_ptr<T> volatile>)
+  {
+      detail::register_shared_ptr(type<shared_ptr<T> >());
+  }
+  
+  template <class T>
+  inline void
+  register_shared_ptr(type<shared_ptr<T> const volatile>)
+  {
+      detail::register_shared_ptr(type<shared_ptr<T> >());
+  }
+  
+  template <class T>
+  registration const& 
+  registry_lookup(type<T&>)
+  {
+      detail::register_shared_ptr(type<T>());
+      return registry::lookup(type_id<T>());
+  }
+  
   template <class T>
   registration const& registered_base<T>::converters
-     = registry::lookup(type_id<T>());
+  = detail::registry_lookup(type<T>());
 }
+
 }}} // namespace boost::python::converter
 
 #endif // REGISTERED_DWA2002710_HPP

include/boost/python/converter/registrations.hpp

@@ -33,7 +33,7 @@ struct rvalue_from_python_chain
 struct BOOST_PYTHON_DECL registration
 {
  public: // member functions
-    explicit registration(type_info);
+    explicit registration(type_info target, bool is_shared_ptr = false);
     
     // Convert the appropriately-typed data to Python
     PyObject* to_python(void const volatile*) const;
@@ -56,7 +56,11 @@ struct BOOST_PYTHON_DECL registration
 
     // The unique to_python converter for the associated C++ type.
     to_python_function_t m_to_python;
-    
+
+    // True iff this type is a shared_ptr.  Needed for special rvalue
+    // from_python handling.
+    const bool is_shared_ptr;
+
 # if BOOST_WORKAROUND(__MWERKS__, BOOST_TESTED_AT(0x3003))
  private:
     void operator=(registration); // This is not defined, and just keeps MWCW happy.
@@ -66,12 +70,13 @@ struct BOOST_PYTHON_DECL registration
 //
 // implementations
 //
-inline registration::registration(type_info target_type)
+inline registration::registration(type_info target_type, bool is_shared_ptr)
     : target_type(target_type)
       , lvalue_chain(0)
       , rvalue_chain(0)
       , m_class_object(0)
       , m_to_python(0)
+      , is_shared_ptr(is_shared_ptr)
 {}
 
 inline bool operator<(registration const& lhs, registration const& rhs)

include/boost/python/converter/registry.hpp

@@ -20,6 +20,10 @@ namespace registry
   // Get the registration corresponding to the type, creating it if necessary
   BOOST_PYTHON_DECL registration const& lookup(type_info);
 
+  // Get the registration corresponding to the type, creating it if
+  // necessary.  Use this first when the type is a shared_ptr.
+  BOOST_PYTHON_DECL registration const& lookup_shared_ptr(type_info);
+
   // Return a pointer to the corresponding registration, if one exists
   BOOST_PYTHON_DECL registration const* query(type_info);
   

include/boost/python/instance_holder.hpp

@@ -23,7 +23,13 @@ struct BOOST_PYTHON_DECL instance_holder : private noncopyable
     // return the next holder in a chain
     instance_holder* next() const;
 
-    virtual void* holds(type_info) = 0;
+    // When the derived holder actually holds by [smart] pointer and
+    // null_ptr_only is set, only report that the type is held when
+    // the pointer is null.  This is needed for proper shared_ptr
+    // support, to prevent holding shared_ptrs from being found when
+    // converting from python so that we can use the conversion method
+    // that always holds the Python object.
+    virtual void* holds(type_info, bool null_ptr_only) = 0;
 
     void install(PyObject* inst) throw();
 

include/boost/python/object/find_instance.hpp

@@ -10,8 +10,11 @@
 namespace boost { namespace python { namespace objects { 
 
 // Given a type_id, find the instance data which corresponds to it, or
-// return 0 in case no such type is held.
-BOOST_PYTHON_DECL void* find_instance_impl(PyObject*, type_info);
+// return 0 in case no such type is held.  If null_shared_ptr_only is
+// true and the type being sought is a shared_ptr, only find an
+// instance if it turns out to be NULL.  Needed for shared_ptr rvalue
+// from_python support.
+BOOST_PYTHON_DECL void* find_instance_impl(PyObject*, type_info, bool null_shared_ptr_only = false);
 
 }}} // namespace boost::python::objects
 

include/boost/python/object/pointer_holder.hpp

@@ -65,7 +65,7 @@ struct pointer_holder : instance_holder
  private: // types
     
  private: // required holder implementation
-    void* holds(type_info);
+    void* holds(type_info, bool null_ptr_only);
     
     template <class T>
     inline void* holds_wrapped(type_info dst_t, wrapper<T>*,T* p)
@@ -99,7 +99,7 @@ struct pointer_holder_back_reference : instance_holder
 #  include BOOST_PP_ITERATE()
 
  private: // required holder implementation
-    void* holds(type_info);
+    void* holds(type_info, bool null_ptr_only);
 
  private: // data members
     Pointer m_p;
@@ -120,9 +120,11 @@ inline pointer_holder_back_reference<Pointer,Value>::pointer_holder_back_referen
 }
 
 template <class Pointer, class Value>
-void* pointer_holder<Pointer, Value>::holds(type_info dst_t)
+void* pointer_holder<Pointer, Value>::holds(type_info dst_t, bool null_ptr_only)
 {
-    if (dst_t == python::type_id<Pointer>())
+    if (dst_t == python::type_id<Pointer>()
+        && !(null_ptr_only && get_pointer(this->m_p))
+    )
         return &this->m_p;
 
     Value* p = get_pointer(this->m_p);
@@ -137,10 +139,12 @@ void* pointer_holder<Pointer, Value>::holds(type_info dst_t)
 }
 
 template <class Pointer, class Value>
-void* pointer_holder_back_reference<Pointer, Value>::holds(type_info dst_t)
+void* pointer_holder_back_reference<Pointer, Value>::holds(type_info dst_t, bool null_ptr_only)
 {
-    if (dst_t == python::type_id<Pointer>())
-        return &this->m_p;
+    if (dst_t == python::type_id<Pointer>()
+        && !(null_ptr_only && get_pointer(this->m_p))
+    )
+	    return &this->m_p;
 
     if (!get_pointer(this->m_p))
         return 0;

include/boost/python/object/value_holder.hpp

@@ -48,7 +48,7 @@ struct value_holder : instance_holder
 #  include BOOST_PP_ITERATE()
 
  private: // required holder implementation
-    void* holds(type_info);
+    void* holds(type_info, bool null_ptr_only);
     
     template <class T>
     inline void* holds_wrapped(type_info dst_t, wrapper<T>*,T* p)
@@ -75,7 +75,7 @@ struct value_holder_back_reference : instance_holder
 #  include BOOST_PP_ITERATE()
 
 private: // required holder implementation
-    void* holds(type_info);
+    void* holds(type_info, bool null_ptr_only);
 
  private: // data members
     Held m_held;
@@ -84,7 +84,7 @@ private: // required holder implementation
 #  undef BOOST_PYTHON_UNFORWARD_LOCAL
 
 template <class Value>
-void* value_holder<Value>::holds(type_info dst_t)
+void* value_holder<Value>::holds(type_info dst_t, bool null_ptr_only)
 {
     if (void* wrapped = holds_wrapped(dst_t, &m_held, &m_held))
         return wrapped;
@@ -96,7 +96,7 @@ void* value_holder<Value>::holds(type_info dst_t)
 
 template <class Value, class Held>
 void* value_holder_back_reference<Value,Held>::holds(
-    type_info dst_t)
+    type_info dst_t, bool null_ptr_only)
 {
     type_info src_t = python::type_id<Value>();
     Value* x = &m_held;

src/converter/from_python.cpp

@@ -43,7 +43,7 @@ BOOST_PYTHON_DECL rvalue_from_python_stage1_data rvalue_from_python_stage1(
 
     // First check to see if it's embedded in an extension class
     // instance, as a special case.
-    data.convertible = objects::find_instance_impl(source, converters.target_type);
+    data.convertible = objects::find_instance_impl(source, converters.target_type, converters.is_shared_ptr);
     if (data.convertible)
     {
         data.construct = 0;

src/converter/registry.cpp

@@ -128,7 +128,7 @@ namespace // <unnamed>
   }
 #endif // BOOST_PYTHON_CONVERTER_REGISTRY_APPLE_MACH_WORKAROUND
 
-  entry* get(type_info type)
+  entry* get(type_info type, bool is_shared_ptr = false)
   {
 #  ifdef BOOST_PYTHON_TRACE_REGISTRY
       registry_t::iterator p = entries().find(entry(type));
@@ -138,7 +138,7 @@ namespace // <unnamed>
                     ? "...NOT found\n" : "...found\n");
 #  endif
       std::pair<registry_t::const_iterator,bool> pos_ins
-          = entries().insert(entry(type));
+          = entries().insert(entry(type,is_shared_ptr));
       
 #  if __MWERKS__ >= 0x3000
       // do a little invariant checking if a change was made
@@ -230,6 +230,11 @@ namespace registry
       return *get(key);
   }
 
+  registration const& lookup_shared_ptr(type_info key)
+  {
+      return *get(key, true);
+  }
+
   registration const* query(type_info type)
   {
       registry_t::iterator p = entries().find(entry(type));

src/object/class.cpp

@@ -407,7 +407,7 @@ namespace objects
   }
 
   BOOST_PYTHON_DECL void*
-  find_instance_impl(PyObject* inst, type_info type)
+  find_instance_impl(PyObject* inst, type_info type, bool null_shared_ptr_only)
   {
       if (inst->ob_type->ob_type != &class_metatype_object)
           return 0;
@@ -416,7 +416,7 @@ namespace objects
 
       for (instance_holder* match = self->objects; match != 0; match = match->next())
       {
-          void* const found = match->holds(type);
+          void* const found = match->holds(type, null_shared_ptr_only);
           if (found)
               return found;
       }

test/Jamfile

@@ -98,6 +98,7 @@ bpl-test crossmod_exception
 [ bpl-test return_arg ]
 [ bpl-test staticmethod ]
 [ bpl-test shared_ptr ]
+[ bpl-test andreas_beyer ]
 [ bpl-test polymorphism ]
 [ bpl-test polymorphism2 ]
 [ bpl-test auto_ptr ]

test/andreas_beyer.cpp

@@ -0,0 +1,58 @@
+#include <boost/python.hpp>
+#include <boost/enable_shared_from_this.hpp>
+#include <boost/shared_ptr.hpp>
+
+using namespace boost;
+
+class A : public enable_shared_from_this<A> {
+ public:
+   A() : val(0) {};
+   int val;
+   typedef shared_ptr<A> A_ptr;
+   A_ptr self() {
+      A_ptr self;
+      self = shared_from_this();
+      return self;
+    }
+
+};
+
+class B {
+  public:
+    B() {
+       a = A::A_ptr(new A());
+    }
+    void set(A::A_ptr a) {
+      this->a = a;
+    }
+    A::A_ptr get() {
+       return a;
+    }
+    A::A_ptr a;
+};
+
+template <class T>
+void hold_python(shared_ptr<T>& x)
+{
+      x = python::extract<shared_ptr<T> >( python::object(x) );
+}
+
+A::A_ptr get_b_a(shared_ptr<B> b)
+{
+    hold_python(b->a);
+    return b->get();
+}
+
+BOOST_PYTHON_MODULE(andreas_beyer_ext) {
+  python::class_<A, noncopyable> ("A")
+    .def("self", &A::self)
+    .def_readwrite("val", &A::val)
+  ;
+  python::register_ptr_to_python< A::A_ptr >();
+ 
+  python::class_<B>("B")
+     .def("set", &B::set)
+//     .def("get", &B::get)
+     .def("get", get_b_a)
+  ;
+}

test/andreas_beyer.py

@@ -0,0 +1,24 @@
+# Copyright David Abrahams 2004. Distributed under the Boost
+# Software License, Version 1.0. (See accompanying
+# file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+'''
+ >>> from andreas_beyer_ext import *
+ >>> b=B()
+ >>> a=b.get() # let b create an A
+ >>> a2=b.get()
+ >>> assert id(a) == id(a2)
+'''
+def run(args = None):
+    import sys
+    import doctest
+
+    if args is not None:
+        sys.argv = args
+    return doctest.testmod(sys.modules.get(__name__))
+    
+if __name__ == '__main__':
+    print "running..."
+    import sys
+    status = run()[0]
+    if (status == 0): print "Done."
+    sys.exit(status)