[Xorp-hackers] xorp callback code replacement?

[Steven Simpson]

Hi Ben,

On 20/09/11 01:10, Ben Greear wrote:
> * callback code is generated by a python script, and results in
>     templated c++ code that I simply cannot understand.
>     This also makes it very hard to add debugging code as well (say,
>     a string name; object so we can identify callbacks generically,
>     or similar.)

Without native variadic templates (C++0X?), I doubt you can get away 
from that.  However, I notice a little redundancy in 
libxorp/callback_nodebug.hh (starting at line 275):

   /**
    * @short Callback object for member methods with 0 dispatch time
    * arguments and 0 bound (stored) arguments.
    */
   template<class R, class O>
   struct XorpMemberCallback0B0 : public XorpCallback0<R>  {
       typedef R (O::*M)() ;
       XorpMemberCallback0B0(O* o, M m)
            : XorpCallback0<R>(),
             _o(o), _m(m) {}
       R dispatch() {
           R r = ((*_o).*_m)();
           return r;
       }
   protected:
       O*  _o;	// Callback's target object
       M   _m;	// Callback's target method
   };

First, the dispatch call creates a local 'r' and then returns it.  
Perhaps it's necessary for some obscure cases, but it should be possible 
to return directly.

Immediately after that, there's a specialised version for returning void:

   /**
    * @short Callback object for void member methods with 0 dispatch time
    * arguments and 0 bound (stored) arguments.
    */
   template<class O>
   struct XorpMemberCallback0B0<void, O>
   : public XorpCallback0<void>  {
       typedef void (O::*M)() ;
       XorpMemberCallback0B0(O* o, M m)
            : XorpCallback0<void>(),
             _o(o), _m(m) {}
       void dispatch() {
           ((*_o).*_m)();
       }
   protected:
       O*  _o;	// Callback's target object
       M   _m;	// Callback's target method
   };

I presume it exists because the (now void) dispatch method can't declare 
the local 'r' for immediate return.  However, I noticed when doing the 
async stuff that you can return a void expression from a void function, 
so it should be safe[*] to use R=void with the more generic version.  If 
the specialised version can be removed, it should cut down on the size 
of the generated code, and maybe cut down on required comprehension.

[*] I'm extrapolating standard behaviour from the observed behaviour of 
a particular compiler, but it seems reasonable to be standard given its 
convenience when used with templates.  Just tried this without error:

$ cat voidret.cc
void m() { }

void n() { return m(); }
$ g++ -std=c++98 -c voidret.cc


> * The callbacks take pointers to objects, and there
>     is virtually no way to cleanly remove the callbacks once
>     added if we are deleting an object that might have a reference
>     held by a callback.

It ought to be possible to have callbacks taking ref_ptrs, instead of, 
or in addition to raw pointers.  I think you'll still have to keep the 
raw-pointer versions, as there will probably be static objects being 
used as receivers somewhere.

Writing XorpMemberCallbackRP0B0 based on XorpMemberCallback0B0, you get:

   /**
    * @short Callback object for member methods with 0 dispatch time
    * arguments and 0 bound (stored) arguments.
    */
   template<class R, class O>
   struct XorpMemberCallbackRP0B0 : public XorpCallback0<R>  {
       typedef R (O::*M)() ;
       XorpMemberCallback0B0(ref_ptr<O>  o, M m)
            : XorpCallback0<R>(),
             _o(o), _m(m) {}
       R dispatch() {
           R r = (_o.get()->*_m)();
           return r;
       }
   protected:
       ref_ptr<O>   _o;	// Callback's target object
       M           _m;	// Callback's target method
   };

For ref_ptrs, you'll have to be careful not to allow the callback 
factory to implicitly make a ref_ptr from a raw one.  IOW, you don't 
want this form:

   static MyObject obj;
   MyObject *ptr =&obj;
   later(callback(ptr,&MyObject::func));

...to create a ref_ptr over obj, because of some unexpected implicit 
conversion.  Not suggesting there is such a conversion, just to watch 
out for it.

Cheers,

Steven