Fully removed convertible() test from to_python converter protocol

dabrahams · dabrahams · commit cfbc1a6b4866 · 2002-11-25T01:57:57.000Z
Added tests for detecting unregistered classes when converting
indirectly to python.


[SVN r16396]
diff --git a/include/boost/python/converter/arg_to_python.hpp b/include/boost/python/converter/arg_to_python.hpp
@@ -29,8 +29,6 @@ template <class T> struct is_object_manager;
 
 namespace detail
 {
-  BOOST_PYTHON_DECL void throw_no_class_registered();
-
   template <class T>
   struct function_arg_to_python : handle<>
   {
@@ -209,8 +207,6 @@ namespace detail
   inline PyObject* reference_arg_to_python<T>::get_object(T& x)
   {
       to_python_indirect<T&,python::detail::make_reference_holder> convert;
-      if (!convert.convertible())
-          throw_no_class_registered();
       return convert(x);
   }
 
@@ -231,9 +227,7 @@ namespace detail
   inline PyObject* pointer_shallow_arg_to_python<Ptr>::get_object(Ptr x)
   {
       to_python_indirect<Ptr,python::detail::make_reference_holder> convert;
-      if (!convert.convertible())
-          throw_no_class_registered();
-      return x ? convert(x) : python::detail::none();
+      return convert(x);
   }
 }
 
diff --git a/include/boost/python/converter/builtin_converters.hpp b/include/boost/python/converter/builtin_converters.hpp
@@ -37,8 +37,6 @@ namespace detail
   // a converter.
   struct builtin_to_python
   {
-      static bool convertible() { return true; }
-
       // This information helps make_getter() decide whether to try to
       // return an internal reference or not. I don't like it much,
       // but it will have to serve for now.
diff --git a/include/boost/python/converter/registrations.hpp b/include/boost/python/converter/registrations.hpp
@@ -35,6 +35,10 @@ struct BOOST_PYTHON_DECL registration
     // Convert the appropriately-typed data to Python
     PyObject* to_python(void const volatile*) const;
 
+    // Return the class object, or raise an appropriate Python
+    // exception if no class has been registered.
+    PyTypeObject* get_class_object() const;
+
  public: // data members. So sue me.
     const python::type_info target_type;
 
@@ -45,7 +49,7 @@ struct BOOST_PYTHON_DECL registration
     rvalue_from_python_chain* rvalue_chain;
     
     // The class object associated with this type
-    PyTypeObject* class_object;
+    PyTypeObject* m_class_object;
 
     // The unique to_python converter for the associated C++ type.
     to_python_function_t m_to_python;
@@ -64,7 +68,7 @@ inline registration::registration(type_info target_type)
       , lvalue_chain(0)
       , rvalue_chain(0)
       , m_to_python(0)
-      , class_object(0)
+      , m_class_object(0)
 {}
 
 inline bool operator<(registration const& lhs, registration const& rhs)
diff --git a/include/boost/python/data_members.hpp b/include/boost/python/data_members.hpp
@@ -36,7 +36,6 @@ namespace detail
           typedef typename Policies::result_converter result_converter;
           typedef typename boost::add_reference<Data>::type source;
           typename mpl::apply1<result_converter,source>::type cr;
-          if (!cr.convertible()) return 0;
         
           if (!policies.precall(args_)) return 0;
 
diff --git a/include/boost/python/enum.hpp b/include/boost/python/enum.hpp
@@ -17,12 +17,15 @@ struct enum_ : public objects::enum_base
 {
     typedef objects::enum_base base;
 
+    // Declare a new enumeration type in the current scope()
     enum_(char const* name);
+
+    // Add a new enumeration value with the given name and value.
     inline enum_<T>& value(char const* name, T);
 
  private:
     static PyObject* to_python(void const* x);
-    static void* convertible(PyObject* obj);
+    static void* convertible_from_python(PyObject* obj);
     static void construct(PyObject* obj, converter::rvalue_from_python_stage1_data* data);
 };
 
@@ -31,32 +34,42 @@ inline enum_<T>::enum_(char const* name)
     : base(
         name
         , &enum_<T>::to_python
-        , &enum_<T>::convertible
+        , &enum_<T>::convertible_from_python
         , &enum_<T>::construct
         , type_id<T>())
 {
 }
 
-// This is the conversion function that gets registered for converting 
+// This is the conversion function that gets registered for converting
+// these enums to Python.
 template <class T>
 PyObject* enum_<T>::to_python(void const* x)
 {
     return base::to_python(
-        converter::registered<T>::converters.class_object
+        converter::registered<T>::converters.m_class_object
         , static_cast<long>(*(T const*)x));
 }
 
+//
+// The following two static functions serve as the elements of an
+// rvalue from_python converter for the enumeration type.
+//
+
+// This checks that a given Python object can be converted to the
+// enumeration type.
 template <class T>
-void* enum_<T>::convertible(PyObject* obj)
+void* enum_<T>::convertible_from_python(PyObject* obj)
 {
     return PyObject_IsInstance(
         obj
         , upcast<PyObject>(
-            converter::registered<T>::converters.class_object))
+            converter::registered<T>::converters.m_class_object))
         
         ? obj : 0;
 }
-      
+
+// Constructs an instance of the enumeration type in the from_python
+// data.
 template <class T>
 void enum_<T>::construct(PyObject* obj, converter::rvalue_from_python_stage1_data* data)
 {
diff --git a/include/boost/python/object/iterator.hpp b/include/boost/python/object/iterator.hpp
@@ -87,7 +87,6 @@ namespace detail
       {
           typedef typename Policies::result_converter result_converter;
           typename mpl::apply1<result_converter,ValueType&>::type cr;
-          if (!cr.convertible()) return 0;
 
           return cr(x);
       }
@@ -96,7 +95,6 @@ namespace detail
       {
           typedef typename Policies::result_converter result_converter;
           typename mpl::apply1<result_converter,ValueType const&>::type cr;
-          if (!cr.convertible()) return 0;
 
           return cr(x);
       }
@@ -150,11 +148,6 @@ namespace detail
 
           to_python_value<iterator_range<NextPolicies,Iterator> > cr;
 
-          // This check is probably redundant, since we ensure the
-          // type is registered above.
-          if (!cr.convertible())
-              return 0;
-
           // Extract x from the first argument
           PyObject* arg0 = PyTuple_GET_ITEM(args_, 0);
           arg_from_python<Target> c0(arg0);
diff --git a/include/boost/python/object/make_instance.hpp b/include/boost/python/object/make_instance.hpp
@@ -20,7 +20,8 @@ struct make_instance
     static PyObject* execute(Arg& x)
     {
         BOOST_STATIC_ASSERT(is_class<T>::value);
-        PyTypeObject* type = converter::registered<T>::converters.class_object;
+        
+        PyTypeObject* type = converter::registered<T>::converters.get_class_object();
 
         PyObject* raw_result = type->tp_alloc(
             type, objects::additional_instance_size<Holder>::value);
diff --git a/include/boost/python/to_python_indirect.hpp b/include/boost/python/to_python_indirect.hpp
@@ -21,7 +21,6 @@ namespace boost { namespace python {
 template <class T, class MakeHolder>
 struct to_python_indirect
 {
-    static bool convertible();
     PyObject* operator()(T ptr) const;
  private:
     static PyTypeObject* type();
@@ -90,16 +89,11 @@ namespace detail
   }
 }
 
-template <class T, class MakeHolder>
-inline bool to_python_indirect<T,MakeHolder>::convertible()
-{
-    BOOST_STATIC_ASSERT(is_pointer<T>::value || is_reference<T>::value);
-    return type() != 0;
-}
-
 template <class T, class MakeHolder>
 inline PyObject* to_python_indirect<T,MakeHolder>::operator()(T x) const
 {
+    BOOST_STATIC_ASSERT(is_pointer<T>::value || is_reference<T>::value);
+        
     PyObject* const null_result = detail::null_pointer_to_none(x, 1L);
     if (null_result != 0)
         return null_result;
diff --git a/include/boost/python/to_python_value.hpp b/include/boost/python/to_python_value.hpp
@@ -27,7 +27,6 @@ namespace detail
           typename add_const<T>::type
       >::type argument_type;
     
-      static bool convertible();
       PyObject* operator()(argument_type) const;
 
       // This information helps make_getter() decide whether to try to
@@ -44,7 +43,6 @@ namespace detail
           typename add_const<T>::type
       >::type argument_type;
     
-      static bool convertible();
       PyObject* operator()(argument_type) const;
 
       // This information helps make_getter() decide whether to try to
@@ -73,24 +71,12 @@ struct to_python_value
 //
 namespace detail
 {
-  template <class T>
-  inline bool registry_to_python_value<T>::convertible()
-  {
-      return converter::registered<argument_type>::converters.to_python != 0;
-  }
-
   template <class T>
   inline PyObject* registry_to_python_value<T>::operator()(argument_type x) const
   {
       return converter::registered<argument_type>::converters.to_python(&x);
   }
 
-  template <class T>
-  inline bool object_manager_to_python_value<T>::convertible()
-  {
-      return true;
-  }
-
   template <class T>
   inline PyObject* object_manager_to_python_value<T>::operator()(argument_type x) const
   {
diff --git a/src/converter/arg_to_python_base.cpp b/src/converter/arg_to_python_base.cpp
@@ -43,14 +43,6 @@ namespace detail
          (converters.to_python(source))
   {
   }
-
-  BOOST_PYTHON_DECL void throw_no_class_registered()
-  {
-      PyErr_SetString(
-          PyExc_TypeError
-          , const_cast<char*>("class not registered for to_python type"));
-      throw_error_already_set();
-  }
 }
 
 }}} // namespace boost::python::converter
diff --git a/src/converter/from_python.cpp b/src/converter/from_python.cpp
@@ -10,6 +10,8 @@
 #include <boost/python/handle.hpp>
 #include <boost/python/detail/raw_pyobject.hpp>
 #include <boost/python/cast.hpp>
+
+#include <boost/lexical_cast.hpp>
 #include <vector>
 #include <algorithm>
 
@@ -244,14 +246,6 @@ BOOST_PYTHON_DECL void* pointer_result_from_python(
     return (lvalue_result_from_python)(source, converters, "pointer");
 }
   
-BOOST_PYTHON_DECL void throw_no_class_registered()
-{
-    PyErr_SetString(
-        PyExc_TypeError
-        , const_cast<char*>("class not registered for to_python type"));
-    throw_error_already_set();
-}
-  
 BOOST_PYTHON_DECL void void_result_from_python(PyObject* o)
 {
     Py_DECREF(expect_non_null(o));
@@ -264,14 +258,12 @@ pytype_check(PyTypeObject* type_, PyObject* source)
 {
     if (!PyObject_IsInstance(source, python::upcast<PyObject>(type_)))
     {
-        handle<> keeper(source);
-        handle<> msg(
-            ::PyString_FromFormat(
-                "Expecting an object of type %s; got an object of type %s instead"
-                , type_->tp_name
-                , source->ob_type->tp_name
-                ));
-        PyErr_SetObject(PyExc_TypeError, msg.get());
+        ::PyErr_Format(
+            PyExc_TypeError
+            , "Expecting an object of type %s; got an object of type %s instead"
+            , type_->tp_name
+            , source->ob_type->tp_name
+            );
         throw_error_already_set();
     }
     return source;
diff --git a/src/converter/registry.cpp b/src/converter/registry.cpp
@@ -6,6 +6,9 @@
 #include <boost/python/converter/registry.hpp>
 #include <boost/python/converter/registrations.hpp>
 #include <boost/python/converter/builtin_converters.hpp>
+
+#include <boost/lexical_cast.hpp>
+
 #include <set>
 #include <stdexcept>
 
@@ -15,6 +18,23 @@
 
 namespace boost { namespace python { namespace converter { 
 
+PyTypeObject* registration::get_class_object() const
+{
+    if (this->m_class_object == 0)
+    {
+        std::string name = lexical_cast<std::string>(this->target_type);
+        ::PyErr_Format(
+            PyExc_TypeError
+            , const_cast<char*>("No Python class registered for C++ class %s")
+            , name.c_str());
+    
+        throw_error_already_set();
+    }
+    
+    return this->m_class_object;
+}
+  
+
 namespace // <unnamed>
 {
   typedef registration entry;
diff --git a/src/object/class.cpp b/src/object/class.cpp
@@ -298,7 +298,7 @@ namespace objects
         converter::registration const* p = converter::registry::query(id);
         return type_handle(
             python::borrowed(
-                python::allow_null(p ? p->class_object : 0))
+                python::allow_null(p ? p->m_class_object : 0))
             );
     }
 
@@ -371,7 +371,7 @@ namespace objects
           converter::registry::lookup(types[0]));
 
       // Class object is leaked, for now
-      converters.class_object = (PyTypeObject*)incref(this->ptr());
+      converters.m_class_object = (PyTypeObject*)incref(this->ptr());
   }
 
   void class_base::set_instance_size(std::size_t instance_size)
diff --git a/src/object/enum.cpp b/src/object/enum.cpp
@@ -169,7 +169,7 @@ enum_base::enum_base(
         = const_cast<converter::registration&>(
             converter::registry::lookup(id));
             
-    converters.class_object = downcast<PyTypeObject>(this->ptr());
+    converters.m_class_object = downcast<PyTypeObject>(this->ptr());
     converter::registry::insert(to_python, id);
     converter::registry::insert(convertible, construct, id);
 }
diff --git a/test/m1.cpp b/test/m1.cpp
@@ -15,6 +15,7 @@
 #include <boost/python/return_value_policy.hpp>
 #include <boost/python/to_python_converter.hpp>
 #include <boost/python/errors.hpp>
+#include <boost/python/manage_new_object.hpp>
 #include <string.h>
 
 // Declare some straightforward extension types
@@ -202,6 +203,8 @@ boost::shared_ptr<A> d_factory() { return boost::shared_ptr<B>(new D); }
 struct Unregistered {};
 Unregistered make_unregistered(int) { return Unregistered(); }
 
+Unregistered* make_unregistered2(int) { return new Unregistered; }
+
 BOOST_PYTHON_MODULE(m1)
 {
     using namespace boost::python;
@@ -226,6 +229,7 @@ BOOST_PYTHON_MODULE(m1)
     def("new_simple", new_simple);
 
     def("make_unregistered", make_unregistered);
+    def("make_unregistered2", make_unregistered2, return_value_policy<manage_new_object>());
 
       // Expose f() in all its variations
     def("f", f);
diff --git a/test/newtest.py b/test/newtest.py

Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,6 @@ template <class T> struct is_object_manager;`
`29`	`29`
`30`	`30`	`namespace detail`
`31`	`31`	`{`
`32`		`- BOOST_PYTHON_DECL void throw_no_class_registered();`
`33`		`-`
`34`	`32`	`template <class T>`
`35`	`33`	`struct function_arg_to_python : handle<>`
`36`	`34`	`{`
`@@ -209,8 +207,6 @@ namespace detail`
`209`	`207`	`inline PyObject* reference_arg_to_python<T>::get_object(T& x)`
`210`	`208`	`{`
`211`	`209`	`to_python_indirect<T&,python::detail::make_reference_holder> convert;`
`212`		`- if (!convert.convertible())`
`213`		`- throw_no_class_registered();`
`214`	`210`	`return convert(x);`
`215`	`211`	`}`
`216`	`212`
`@@ -231,9 +227,7 @@ namespace detail`
`231`	`227`	`inline PyObject* pointer_shallow_arg_to_python<Ptr>::get_object(Ptr x)`
`232`	`228`	`{`
`233`	`229`	`to_python_indirect<Ptr,python::detail::make_reference_holder> convert;`
`234`		`- if (!convert.convertible())`
`235`		`- throw_no_class_registered();`
`236`		`- return x ? convert(x) : python::detail::none();`
	`230`	`+ return convert(x);`
`237`	`231`	`}`
`238`	`232`	`}`
`239`	`233`
Original file line number	Diff line number	Diff line change
`@@ -37,8 +37,6 @@ namespace detail`
`37`	`37`	`// a converter.`
`38`	`38`	`struct builtin_to_python`
`39`	`39`	`{`
`40`		`- static bool convertible() { return true; }`
`41`		`-`
`42`	`40`	`// This information helps make_getter() decide whether to try to`
`43`	`41`	`// return an internal reference or not. I don't like it much,`
`44`	`42`	`// but it will have to serve for now.`
Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,6 @@ namespace detail`
`87`	`87`	`{`
`88`	`88`	`typedef typename Policies::result_converter result_converter;`
`89`	`89`	`typename mpl::apply1<result_converter,ValueType&>::type cr;`
`90`		`- if (!cr.convertible()) return 0;`
`91`	`90`
`92`	`91`	`return cr(x);`
`93`	`92`	`}`
`@@ -96,7 +95,6 @@ namespace detail`
`96`	`95`	`{`
`97`	`96`	`typedef typename Policies::result_converter result_converter;`
`98`	`97`	`typename mpl::apply1<result_converter,ValueType const&>::type cr;`
`99`		`- if (!cr.convertible()) return 0;`
`100`	`98`
`101`	`99`	`return cr(x);`
`102`	`100`	`}`
`@@ -150,11 +148,6 @@ namespace detail`
`150`	`148`
`151`	`149`	`to_python_value<iterator_range<NextPolicies,Iterator> > cr;`
`152`	`150`
`153`		`- // This check is probably redundant, since we ensure the`
`154`		`- // type is registered above.`
`155`		`- if (!cr.convertible())`
`156`		`- return 0;`
`157`		`-`
`158`	`151`	`// Extract x from the first argument`
`159`	`152`	`PyObject* arg0 = PyTuple_GET_ITEM(args_, 0);`
`160`	`153`	`arg_from_python<Target> c0(arg0);`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,8 @@ struct make_instance`
`20`	`20`	`static PyObject* execute(Arg& x)`
`21`	`21`	`{`
`22`	`22`	`BOOST_STATIC_ASSERT(is_class<T>::value);`
`23`		`- PyTypeObject* type = converter::registered<T>::converters.class_object;`
	`23`	`+`
	`24`	`+ PyTypeObject* type = converter::registered<T>::converters.get_class_object();`
`24`	`25`
`25`	`26`	`PyObject* raw_result = type->tp_alloc(`
`26`	`27`	`type, objects::additional_instance_size<Holder>::value);`
Original file line number	Diff line number	Diff line change
`@@ -43,14 +43,6 @@ namespace detail`
`43`	`43`	`(converters.to_python(source))`
`44`	`44`	`{`
`45`	`45`	`}`
`46`		`-`
`47`		`- BOOST_PYTHON_DECL void throw_no_class_registered()`
`48`		`- {`
`49`		`- PyErr_SetString(`
`50`		`- PyExc_TypeError`
`51`		`- , const_cast<char*>("class not registered for to_python type"));`
`52`		`- throw_error_already_set();`
`53`		`- }`
`54`	`46`	`}`
`55`	`47`
`56`	`48`	`}}} // namespace boost::python::converter`