Obviate method_finder methods

Obviate method_finder methods

The inheritance of various Translator::method_finder functions was a
comparatively fragile mess.  A helper class mfp_baseclass is able to
figure out the proper baseclass for a given member function pointer.
That allows removing the method_finder functions from the various
Translator-derived classes.

[lemzwerg, 2020-04-19 05:53:34]

Nice!  I don't really understand the C++ wizardry, but having to type less
macros in the normal code is certainly beneficial.

[dak, 2020-04-19 07:56:36]

On 2020/04/19 05:53:34, lemzwerg wrote:
> Nice!  I don't really understand the C++ wizardry, but having to type less
> macros in the normal code is certainly beneficial.

The wizardry is comparatively confined and mainly consists of a more polished
version of the mfp_baseclass thing that I needed to address Dan's complaints
about issue 5916 (which will need to get rebased on this more elaborate
version).  The advantage, of course, is that the whole problem underlying issue
5600 that kept Clang from working (and likely would have stopped GCC at some
point of time) before Jonas found a solution: gone.

The fundamental problem is that something like
&Global_context::create_context_from_event (a member function pointer to an
inherited function) is not of type void (Global_context::*)(SCM) but of type
void (Context::*)(SCM), and in the context of template argument matching, you
need to match the type exactly.

The basic C++ wizardry used here would already have worked in C++98 but would
have required a number of more specialised versions of mfp_baseclass (for lack
of template parameter packs).  I just failed to come up with it before Dan asked
for wart removal in issue 5916.

The disadvantage, of course is "I don't really understand the C++ wizardry". 
C++11 has appropriated Perl's capacity of reading like line noise, but in
contrast to Perl, you never really get to the stage where this line noise stops
causing headaches.

At least this one saves a lot of other headaches.  And mfp_baseclass does a
well-defined job in a manner and with an interface typical to C++11.  And it's
few lines of headache in a single place rather than something spread over all of
the source code.

[hahnjo, 2020-04-19 09:22:44]

A huge THANK YOU for this cleanup and getting rid of my 'TRANSLATOR_INHERIT'
workaround. One request for using C++ style inline.

On the topic of 'C++ wizardry': While not completely straight-forward I think
mfp_baseclass is small enough to get your head around it if needed. I mean half
of it is remove_pointer which is part of type_traits and one of the prime
examples when starting with template programming. The rest is hard to come up
with (kudos that you did!), but that's what you get for hiding static members in
derived classes.

https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh
File lily/include/callback.hh (right):

https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback...
lily/include/callback.hh:166: typedef typename remove_pointer<decltype(strip_mfp
(static_cast<T> (nullptr)))>::type type;
I'd consider 'using type = ' instead which is more C++ style

[dak, 2020-04-19 09:39:00]

https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh
File lily/include/callback.hh (right):

https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback...
lily/include/callback.hh:166: typedef typename remove_pointer<decltype(strip_mfp
(static_cast<T> (nullptr)))>::type type;
On 2020/04/19 09:22:44, hahnjo wrote:
> I'd consider 'using type = ' instead which is more C++ style

I think you overestimate my C++ fu.  Can you spell this out?

[hahnjo, 2020-04-19 09:55:03]

On 2020/04/19 09:39:00, dak wrote:
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh
> File lily/include/callback.hh (right):
> 
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback...
> lily/include/callback.hh:166: typedef typename
remove_pointer<decltype(strip_mfp
> (static_cast<T> (nullptr)))>::type type;
> On 2020/04/19 09:22:44, hahnjo wrote:
> > I'd consider 'using type = ' instead which is more C++ style
> 
> I think you overestimate my C++ fu.  Can you spell this out?

using type = typename remove_pointer<decltype(strip_mfp (static_cast<T>
(nullptr)))>::type;
to declare the type alias: https://en.cppreference.com/w/cpp/language/type_alias

[dak, 2020-04-19 10:55:52]

On 2020/04/19 09:55:03, hahnjo wrote:
> On 2020/04/19 09:39:00, dak wrote:
> >
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh
> > File lily/include/callback.hh (right):
> > 
> >
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback...
> > lily/include/callback.hh:166: typedef typename
> remove_pointer<decltype(strip_mfp
> > (static_cast<T> (nullptr)))>::type type;
> > On 2020/04/19 09:22:44, hahnjo wrote:
> > > I'd consider 'using type = ' instead which is more C++ style
> > 
> > I think you overestimate my C++ fu.  Can you spell this out?
> 
> using type = typename remove_pointer<decltype(strip_mfp (static_cast<T>
> (nullptr)))>::type;
> to declare the type alias:
https://en.cppreference.com/w/cpp/language/type_alias

I see.  Basically squeezing in another piece of syntax where it has no sensible
place in order to have a variant of typedef that also works for template
specialisation of types.  Or in other words, more C++ style.  I have the vague
impression that the ability to do template specialisation on typedefs would also
be able to solve part of the job here in a more elegant manner but right now my
brain objects to more C++ style.  So I'll confine myself to the change you
suggest for now.  Will take probably half the day to get to the next patch as I
am also doing some other reorganisation.

Thanks!

[hahnjo, 2020-04-19 11:03:35]

On 2020/04/19 10:55:52, dak wrote:
> On 2020/04/19 09:55:03, hahnjo wrote:
> > On 2020/04/19 09:39:00, dak wrote:
> > >
> >
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh
> > > File lily/include/callback.hh (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback...
> > > lily/include/callback.hh:166: typedef typename
> > remove_pointer<decltype(strip_mfp
> > > (static_cast<T> (nullptr)))>::type type;
> > > On 2020/04/19 09:22:44, hahnjo wrote:
> > > > I'd consider 'using type = ' instead which is more C++ style
> > > 
> > > I think you overestimate my C++ fu.  Can you spell this out?
> > 
> > using type = typename remove_pointer<decltype(strip_mfp (static_cast<T>
> > (nullptr)))>::type;
> > to declare the type alias:
> https://en.cppreference.com/w/cpp/language/type_alias
> 
> I see.  Basically squeezing in another piece of syntax where it has no
sensible
> place in order to have a variant of typedef that also works for template
> specialisation of types.  Or in other words, more C++ style.  I have the vague
> impression that the ability to do template specialisation on typedefs would
also
> be able to solve part of the job here in a more elegant manner but right now
my
> brain objects to more C++ style.

Yes, I had the same feeling. However I think that the current solution works
because the template arguments are deduced from the function argument. I don't
currently see a way to make this work with template <> using type =, but that
would certainly be nice (and remove the need of using decltype and
remove_pointer).

[dak, 2020-04-19 12:24:32]

On 2020/04/19 11:03:35, hahnjo wrote:
> On 2020/04/19 10:55:52, dak wrote:
> I have the vague
> > impression that the ability to do template specialisation on typedefs would
> also
> > be able to solve part of the job here in a more elegant manner but right now
> my
> > brain objects to more C++ style.
> 
> Yes, I had the same feeling. However I think that the current solution works
> because the template arguments are deduced from the function argument. I don't
> currently see a way to make this work with template <> using type =, but that
> would certainly be nice (and remove the need of using decltype and
> remove_pointer).

The principal problem is that the trampoline instantiation requires the member
function pointer as a non-type template parameter, and the type of the non-type
template parameter is not known in general.  So this becomes a hen-and-egg
problem.  Template specialisation could possibly weasel around it but I am not
sure about it.  It certainly is suspicious that the otherwise rather
comprehensive <type_traits> relying on such techniques has nothing in its
toolbox for that task.

[dak, 2020-04-20 19:36:39]

Polish and extend tools in callback.hh

[Dan Eble, 2020-04-20 23:26:07]

On 2020/04/20 19:36:39, dak wrote:
> Polish and extend tools in callback.hh

LGTM.

I have a hunch that the MFPn_WRAP macros could be turned into something more
typical of C++, but I don't blame you for resting at this point.

[dak, 2020-04-21 00:43:02]

On 2020/04/20 23:26:07, Dan Eble wrote:
> On 2020/04/20 19:36:39, dak wrote:
> > Polish and extend tools in callback.hh
> 
> LGTM.
> 
> I have a hunch that the MFPn_WRAP macros could be turned into something more
> typical of C++, but I don't blame you for resting at this point.

Not without a lot of effort since MFP1_WRAP and MFP2_WRAP rely on "trampoline"
in the current lexical scope doing something sensible.  They are unclean macros.
 So one would have to overhaul the whole trampoline concept to collect all
trampolines in one scope (either :: or Callback?::) and scatter specialisations
over the files that want them.  A lot of work for what does not really look like
a whole lot of gain.

[dak, 2020-04-21 00:56:30]

On 2020/04/21 00:43:02, dak wrote:
> On 2020/04/20 23:26:07, Dan Eble wrote:
> > On 2020/04/20 19:36:39, dak wrote:
> > > Polish and extend tools in callback.hh
> > 
> > LGTM.
> > 
> > I have a hunch that the MFPn_WRAP macros could be turned into something more
> > typical of C++, but I don't blame you for resting at this point.
> 
> Not without a lot of effort since MFP1_WRAP and MFP2_WRAP rely on "trampoline"
> in the current lexical scope doing something sensible.  They are unclean
macros.
>  So one would have to overhaul the whole trampoline concept to collect all
> trampolines in one scope (either :: or Callback?::) and scatter
specialisations
> over the files that want them.  A lot of work for what does not really look
like
> a whole lot of gain.

Also if you were thinking of turning MFP0_WRAP into a _function_ taking a
pointer to member function argument, that will not work since the whole
trampoline creation concept relies on the particular pointer to member being a
template argument and thus fixed.  There is a separate trampoline generated for
each member function.