[C++-sig] Wrapping opaque pointers returned from API function s

[Gottfried Ganßauge]

> Gottfried,
>
> This is a pretty nice implementation!
Thanks.

>
> However, there are a few slight problems that will show up on very
> conformant C++ implementations: firstly, pointer_converter<P>::dealloc
> has C++ linkage, so you can't stick it in the tp_dealloc slot, which
> has to be a function pointer with 'C' linkage.  Second, a few things
Thought about that myself, but as my compiiler (msvc) didn't complain ...

> you are doing with pointer_converter depend on it having an initial
> section which is layout-compatible with PyObject.  Unfortunately, the
> C++ standard doesn't guarantee anything about object layout for
> classes with bases, constructors, or private members (non-POD types).
Didn't know that. Just for curiosity: Are there real implementations making
a difference here?

> Third, your use of the typename keyword in the declaration of the 'x'
> member is illegal.  You can only use typename for dependent types;
> paradoxically, template arguments themselves are not dependent.  You
> also left out a typename on the execute function.
I'll make it better the next time. promised :-)

>
> Finally, I think your DEFINE_POINTER_CONVERTER macro is likely to make
> a Python type name containing illegal characters like '*', though I
> guess you are passing a typedef here so it's probably not a big deal.
As I said: I wasn't really happy with that macro; I like your your solution
much better.

>
> Allow me to suggest the following modification (untested):
I like it - especially as it does away with my macros - , and I took the
freedom to make it work (on MSVC that is, see below):

>
> <------------->
> // convert_opaque_pointer --
> //
> // usage: convert_opaque_pointer<pointer_type>("name")
> //
> // registers to- and from- python conversions for a type pointer_type,
> // and a corresponding Python type called "name".
> //
> namespace detail
                     ^^^^^
Both mscv compilers (6.0 and 7) agree that this is ambigous (probably with
boost::python::detail !?). The complaint, alas, occurs when using
detail::dealloc!

> {
>     extern "C" inline void dealloc(PyObject* self)
                       ^^^^^
We're using a pointer to this function; therefore this function can't be
expanded inline, right?

>     {
>         PyObject_Del(self);
>     }
> }
>
> template <class Pointer>
> struct convert_opaque_pointer
>     : ::boost::python::to_python_converter<
>           Pointer, convert_opaque_pointer<Pointer> >
> {
>     // This is a POD so we can use PyObject_Del on it, for example.
I see your point, but msvc doesn't accept your upcast below; and anyway; who
guarantees that PyObject won't be declared differently in the next python
release?
What about struct instance : public PyObject { Pointer x; }; ?

>     struct instance
>     {
>         PyObject_HEAD
>         Pointer x;
>     };
>
>     convert_opaque_pointer(char const* name)
>     {
>         type_object.tp_name = name;
                                               ^^^^^ should be
const_cast<char *> (name), as tp_name is declared char *
>
>         boost::python::lvalue_from_pytype<
>             opaque_pointer_converter, &opaque_pointer_converter::type
               ^^^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^^^^^^^^^^^^^^^
I take it you mean
convert_opaque_pointer<Pointer>,
convert_opaque_pointer<Pointer>::type_object

>         >();
>     }
>
>     static PyObject* convert(Pointer x)
>     {
>         instance *o = PyObject_New (instance, &type_object);
>         o->x = x;
>         return boost::python::detail::upcast<PyObject>(o);
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
MSVC6 produces a compiler error here (error C2780: 'Target *__cdecl
boost::python::detail::upcast(Source *,int *,int *,Target *)' : expected 4
arguments - 1 supported), but it works, when I derive instance publicly from
PyObject.
>     }
>
>     static typename boost::remove_pointer<Pointer>::type&
>     execute(instance &p_)
>     {
>         return *p_.x;
>     }
>
>     static PyTypeObject type_object;
> };
>
> template <class Pointer>
> PyTypeObject convert_opaque_pointer<Pointer>::type_object =
> {
>     PyObject_HEAD_INIT(NULL)
>     0,
>     0,
>     sizeof(typename convert_opaque_pointer<Pointer>::instance),
>     0,
>     detail::dealloc
> };
> <------------->
>
> After we settle on something, I'd like to put it in the library, or at
I would be more than glad about that.

> least in the FAQ.
>
Finally my edition of your implementation:
// convert_opaque_pointer --
//
// usage: convert_opaque_pointer<pointer_type>("name")
//
// registers to- and from- python conversions for a type pointer_type,
// and a corresponding Python type called "name".
//
namespace cop_detail
{
    extern "C" void dealloc(PyObject* self)
    {
        PyObject_Del(self);
    }
}

template <class Pointer>
struct convert_opaque_pointer
    : ::boost::python::to_python_converter<
          Pointer, convert_opaque_pointer<Pointer> >
{
    // This is a POD so we can use PyObject_Del on it, for example.
    struct instance : public PyObject
    {
        Pointer x;
    };

    convert_opaque_pointer(char const* name)
    {
        type_object.tp_name = const_cast<char *> (name);

        boost::python::lvalue_from_pytype<
            convert_opaque_pointer<Pointer>,
           &convert_opaque_pointer<Pointer>::type_object
        >();
    }

    static PyObject* convert(Pointer x)
    {
        instance *o = PyObject_New (instance, &type_object);

        o->x = x;
        return o;
    }

    static typename boost::remove_pointer<Pointer>::type&
    execute(instance &p_)
    {
        return *p_.x;
    }

    static PyTypeObject type_object;
};

template <class Pointer>
PyTypeObject convert_opaque_pointer<Pointer>::type_object =
{
    PyObject_HEAD_INIT(NULL)
    0,
    0,
    sizeof(typename convert_opaque_pointer<Pointer>::instance),
    0,
    cop_detail::dealloc
};

Cheers,

Gottfried