On 2020/04/18 10:30:27, hanwenn wrote: > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh > File lily/include/listener.hh (left): > > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh#oldcode140 > lily/include/listener.hh:140: #define GET_LISTENER(cl, proc) get_listener > (Callback_wrapper::make_smob<Listener::trampoline<cl, &cl::proc> > ()) > IMO, the problem is that the macro scopes 'proc' to come from 'cl', which is > magical. Couldn't we change the calling convention to be > > ctx->GET_LISTENER(&SomeContext::func)
Nope. For one thing, you should be happy Smob_core no longer needs to provide a member function just for the sake of Listener, making Listener less generic and introducing a compile-time dependency. For another, it requires the call site to spell out the type of the expression ctx. And the third is that the implementation still needs to derive SomeContext from &SomeContext::func in order to meet the requirements of the Callback_wrapper cell created. > if we call this from within a SomeContext method itself, it could be > > GET_LISTENER(&func) No, it couldn't. C++ member function pointer syntax cannot be done in that manner. > If so, would we need a macro at all? The main purpose of the macro is not providing a wrapper for the Listener creator, but rather for creating the template arguments for the Callback_wrapper magic. That one's the hardcore bit. The Listener type just curries two SCM values. https://codereview.appspot.com/549890043/