GNOME Bugzilla – Bug 530416
Adding default constructor to Glib::ArrayHandle<...>
Last modified: 2011-02-21 11:33:50 UTC
Please describe the problem: Trying to wrap Gst::Element vfunc "get_query_types" (which ideally would return a Glib::ArrayHandle<Gst::QueryType>), I get the following compile error: element.cc: In member function 'virtual Glib::ArrayHandle<Gst::QueryType, Glib::Container_Helpers::TypeTraits<Gst::QueryType> > Gst::Element::get_query_types_vfunc()': element.cc:1440: error: no matching function for call to 'Glib::ArrayHandle<Gst::QueryType, Glib::Container_Helpers::TypeTraits<Gst::QueryType> >::ArrayHandle()' caused by the last line (the second return) in the generated vfunc: Glib::ArrayHandle<QueryType> Gst::Element::get_query_types_vfunc() { BaseClassType *const base = static_cast<BaseClassType*>( g_type_class_peek_parent(G_OBJECT_GET_CLASS(gobject_)) // Get the parent class of the object class (The original underlying C class). ); if(base && base->get_query_types) return Glib::ArrayHandle<QueryType>((QueryType*)((*base->get_query_types)(gobj())), Glib::OWNERSHIP_SHALLOW); typedef Glib::ArrayHandle<QueryType> RType; return RType(); } It looks like Glib::ArrayHandle<...> needs a default constructor in order for the second return to be compiled correctly. Would the attached patch be ok? It allows clean compilation when this vfunc is wrapped (assuming, of course, that this vfunc should be wrapped :-) Steps to reproduce: Actual results: Expected results: Does this happen every time? Other information:
Created attachment 110056 [details] [review] Patch to add default constructor to Glib::ArrayHandle<>
(In reply to comment #0) > It looks like Glib::ArrayHandle<...> needs a default constructor in order for > the second return to be compiled correctly. Would the attached patch be ok? > It allows clean compilation when this vfunc is wrapped (assuming, of course, > that this vfunc should be wrapped :-) > Correction: I think it should be wrapped, but I could be wrong.
The ArrayHandle (and other intermediate types) are a bit mysterious. I want to be very careful about making any changes to them. And I don't want to encourage people to use them directly - which would seem more possible if there was a default constructor. I think I'd rather use RType(NULL) in that code, though you'll probably need my help to make that code be generated. Please don't wrap this vfunc yet, but leave a TODO there for me.
(In reply to comment #3) > The ArrayHandle (and other intermediate types) are a bit mysterious. I want to > be very careful about making any changes to them. And I don't want to encourage > people to use them directly - which would seem more possible if there was a > default constructor. > > I think I'd rather use RType(NULL) in that code, though you'll probably need my > help to make that code be generated. Please don't wrap this vfunc yet, but > leave a TODO there for me. > Okay. Thanks.
Created attachment 124323 [details] [review] Patch to add optional final return expression argument to _WRAP_VFUNC This patch makes it possible to specify a custom return expression for the vfunc callback in case the base class does not implement the vfunc (for vfuncs that return something). There are also some gstreamermm bool vfunc callbacks that cause problems because they return false (an unsuccessful indication) instead of true (a succesful indication) when the base class does not implement the bool vfunc because of the "typedef gboolean RType; return RType()" lines in the bool vfunc callback (boiling down to false). This happens often with classes like GstBaseSrc and GstBaseSink which don't always implement the bool vfuncs. Quite a few plug-ins rely on these base classes and don't work properly when used unless these callbacks are amended to return true. Both the bug and the problem described above would be resolved with the patch. _WRAP_VFUNC would work normally but additionally it could be used in the following way (taken from basesink.hg in gstreamermm): _WRAP_VFUNC(bool start(), "start", true) The "true" argument would generate the code below: gboolean BaseSink_Class::start_vfunc_callback(GstBaseSink* self) { ... // Call the original underlying C function: if(base && base->start) return (*base->start)(self); return true; } instead of the original (without the `true' argument): gboolean BaseSink_Class::start_vfunc_callback(GstBaseSink* self) { ... // Call the original underlying C function: if(base && base->start) return (*base->start)(self); typedef gboolean RType; return RType(); }
Created attachment 124329 [details] [review] Replcement patch that generates both the callback and implementation with the optional return Oops. I think that the implementation should be treated the same as the callback.
Wait. The implementation and the callback return different types so they have to be treated differently (I'm not thinking straight this late). Maybe a patch that allows two optional returns (one for the implementation and one for the callback) would work better. Would this be a good idea? If so, I can submit one later in the day (afternoon for me).
Created attachment 124378 [details] [review] Patch to add two optional final return expression arguments to _WRAP_VFUNC This patch adds two optional arguments (one for the vfunc implementation return and the other for the callback return). _WRAP_VFUNC could be used in the following way: _WRAP_VFUNC(cppProtoType,"name",[refreturn],[ifdef],[implementationReturnExpression],[callbackReturnExpression]) The optional arguments can go in any order, and any of them (the optional ones) may be omitted. To provide a callbackReturnExpression and not an implementationReturnExpression an empty implementationReturnExpression must be specified first and then the callbackReturnExpression as in the following: _WRAP_VFUNC(bool vfunc(), "vfunc",,true | true) Which would replace the "typedef <cType> RType; return RType();" lines in the gboolean vfunc callback with "return true | true;". If only one return expression is specified like so: _WRAP_VFUNC(bool vfunc(), "vfunc", true) It is assumed to be for the implementation of the vfunc and not for the callback. I understand that this may be a weird patch, but it would allow certain problems with gstreamermm vfuncs to be fixed (if they are wrapped). If it is needed for gstreamermm (if at any time the vfuncs want to be wrapped), this patch may be used then.
Handwriting the vfuncs is a lot easier. Closing.
The reason I closed this bug is because I thought the patch may be confusing or useless. I think I'll let the glibmm make that determination. Re-opening and marking as enhancement.
We use std::vector now anyway, via the new vector_utils, so I guess this isn't useful anymore. Thanks anyway. Actually closing this now.