Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by address@hidden)

[dak]

On 2020/04/18 05:40:53, lemzwerg wrote:
> LGTM - but I don't know the internals well enough to really decide
that.

It's really not as much about the internals rather than about how to
express their use.  GET_LISTENER provides an SCM-callable memoized
function port into a C++ member function.  The last iteration of this
code in issue 4357 had to add the type argument to the GET_LISTENER
macro and I was comparatively unhappy about the resulting syntax.  The
change was akin to

-  add_listener (GET_LISTENER (new_context->unset_property_from_event),
+  add_listener (new_context->GET_LISTENER (Context,
unset_property_from_event),

As you can see, I had to split the previous single argument into two and
separately specify the type of the first.  The items that this technique
needs to process are
Context, &Context::unset_property_from_event for creating an
SCM-accessible memoized trampoline function and new_context for
generating an SCM listener port to a specific Smob structure.
> 
> What about using two macros instead of one?  The first would take a
context as a
> first argument (as it does now), the second one uses 'this' in the
macro body,
> omitting that as a macro argument.  The former seems to be a rarer
case than the
> latter.

Well, the problem is that having GET_LISTENER (... and GET_LISTENER
(this, ... does not really take significantly more space than
GET_LISTENER (... and GET_SELF_LISTENER (... while being less clear
about what is happening.

Similarly for, say, GET_OUTSIDE_LISTENER vs GET_LISTENER (if you try to
give the explicit listener the longer name).

I agree with the sentiment, but I fail to come up with a naming choice
that does not make the cure worse than the problem.

https://codereview.appspot.com/549890043/