[C++-sig] Re: Python + Boost Python V2 + downcasting

[David Abrahams]

"Nicolas Lelong" <n_lelong at hotmail.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 at boost-consulting.com * http://www.boost-consulting.com
Boost support, enhancements, training, and commercial distribution