"Nicolas Lelong" <n_lelong@[...].com>  writes:

>  David Abrahams wrote:
> 
> >[...]
> >
> > Ultimately, I think the best solution is going to be to add a new kind
> > of ReturnValuePolicy which uses typeid() on the pointer to get the
> > most-derived class, looks up the corresponding Python class in the
> > converter::registry, and builds a pointer_holder around it using the
> > appropriate Python class type. Hmm, looking carefully, this might not
> > require a new ReturnValuePolicy, and could be as simple as making some
> > judicious changes to boost/python/object/make_instance.hpp.**
> >
> 
>  OK Dave,
>  I've decided to go for it, and I think I've made up the kind of changes
>  needed in 'boost/python/object/make_instance.hpp'.
> 
>  I replaced the following line from 'PyObject* make_instance::execute(Arg&
>  x)' :
>      PyTypeObject* type = converter::registered<T>::converters.class_object;
>  with:
>      PyTypeObject* type = query_most_derived_PyTypeObject( x );
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

FWIW, unqualified calls are banned unless you intend to allow
customization via Koenig Lookup.

>  Having some news methods in make_instance:
> 
>  template <class Arg>
>  static dynamic_id_t query_most_derived_dynamic_id_t(Arg& x)
>  {
>      typedef typename dynamic_id_generator<T>::type generator;
>      return generator::execute( (void*)&(*x) );

You shouldn't use a cast here. Any object pointer is implicitly
convertible to void*.

>  }
> 
>  static dynamic_id_t query_most_derived_dynamic_id_t(reference_wrapper<T
>  const> x)
>  {
>      typedef typename dynamic_id_generator<T>::type generator;
>      return generator::execute( (void*)x.get_pointer() );
>  }

Are you sure you don't need an overload for reference_wrapper<T>  as
well? And, is a reference_wrapper check really needed? Don't I unwrap
all reference_wrappers before they arrive here? Hmm, maybe not...

>  template <class Arg>
>  static PyTypeObject* query_most_derived_PyTypeObject(Arg& x)
>  {
>      if (is_polymorphic<T>::value)
>      {
>          dynamic_id_t dynamic_id = query_most_derived_dynamic_id_t( x );
>          converter::registration const* registration_data =
>  converter::registry::query( dynamic_id.second );
>          return registration_data->class_object;

This won't work. If the most-derived class is not registered,
registration_data will be 0. You need a fallback. Also, it seems like
overkill since dynamic_id_generator<T>  is already checking for
is_polymorphic... and it's doing a lot more than that in order to
avoid using dynamic_cast on non-polymorphic types.


>      }
>      else
>      {
>          return converter::registered<T>::converters.class_object;
>      }
>  }

Why not implement the above as, simply:

    template <class T> 
    type_info dynamic_id(T const volatile* x, ...)
    {
        // const_cast to T* used to work around broken compilers which
        // leave cv-qualifications on type_info.
        return boost::python::type_info(typeid(const_cast<T*> (x)));
    }
    
    template <class Arg> 
    static PyTypeObject* query_most_derived_PyTypeObject(Arg& x)
    {
        typedef typename unforward<Arg> ::type arg_type;
        arg_type a = x;
        converter::registration const* registration_data
           = converter::registry::query( dynamic_id(&a) );

        if (registration_data && registration_data-> class_object)
            return registration_data-> class_object;
        else
            return converter::registered<T> ::converters.class_object;
    }

(or something like that)?

>  It seems to work fine on a few examples I tried here - I must admit that
>  I've not run the whole test suite, but what do you think of it ?!

I think you're barking up the right tree, but it still needs some
work. Very exciting!

-- 
                       David Abrahams
   dave@[...].com * http://www.boost-consulting.com
Boost support, enhancements, training, and commercial distribution


_______________________________________________
C++-sig mailing list
C++-sig@[...].org
http://mail.python.org/mailman/listinfo/c++-sig