Re: Fix hidden member templates in derived classes (issue 559250043 by address@hidden)

[jonas . hahnfeld]

Reviewers: lemzwerg,

Message:
On 2019/11/14 08:45:44, lemzwerg wrote:
> Wait, this tiny patch makes lilypond compilable with clang, passing
the whole
> test suite?

You're quite fast with looking at patches, I wasn't even finished with
pointing you to this one :D
And yes, this works for me, at least on Linux (didn't test FreeBSD yet
and don't own a Mac). Plus it passes a full check with a test-baseline
generated by GCC, so I'm pretty confident it's also working correctly.

Description:
Fix hidden member templates in derived classes

According to Clang's interpretation of the C++ standard, "member
functions in the derived class override and/or hide member functions
with the same name and parameter types in a base class". This seems
to also hold for templated member functions and even if the code has
a using-declaration of the base class member.
This affects LilyPond's implementation of the method_finder template.
Luckily, the problem can be solved by just re-declaring the base
template specializations in each derived class instead of 'using' them.
The only drawback is that this doesn't work out-of-the-box for deeper
inheritance hierarchies. However, this only affects three ligature
engravers so I think it's worth the maintenance to just encode the
transitive inheritance manually.

Please review this at https://codereview.appspot.com/559250043/

Affected files (+10, -6 lines):
  M lily/include/translator.hh
  M lily/kievan-ligature-engraver.cc
  M lily/mensural-ligature-engraver.cc
  M lily/vaticana-ligature-engraver.cc


Index: lily/include/translator.hh
diff --git a/lily/include/translator.hh b/lily/include/translator.hh
index  
63f9ed78acd42a29d0af33f9f88b582b3c15415c..54e289771636b14276158745141ebbe9a949dd23  
100644
--- a/lily/include/translator.hh
+++ b/lily/include/translator.hh
@@ -75,12 +75,16 @@ Translator_creator::allocate (Context *ctx)
   public:                                                               \
   DECLARE_CLASSNAME (NAME);                                             \
   virtual void fetch_precomputable_methods (SCM methods[]);             \
-  DECLARE_TRANSLATOR_CALLBACKS (NAME);                                  \
-  TRANSLATOR_INHERIT (Translator);                                      \
+  template <void (Translator::*)()>                                     \
+  static SCM method_finder ()                                           \
+  {                                                                     \
+    return SCM_UNDEFINED;                                               \
+  }                                                                     \
+  DECLARE_TRANSLATOR_CALLBACKS (NAME)                                   \
   /* end #define */

 #define TRANSLATOR_INHERIT(BASE)                                        \
-  using BASE::method_finder
+  DECLARE_TRANSLATOR_CALLBACKS (BASE)

 #define DECLARE_TRANSLATOR_CALLBACKS(NAME)                              \
   template <void (NAME::*mf)()>                                         \
@@ -199,9 +203,6 @@ protected:                      // should be private.

   // Fallback for non-overriden callbacks for which &T::x degrades to
   // &Translator::x
-  template <void (Translator::*)()>
-  static SCM
-  method_finder () { return SCM_UNDEFINED; }

   virtual void derived_mark () const;
   static SCM event_class_symbol (const char *ev_class);
Index: lily/kievan-ligature-engraver.cc
diff --git a/lily/kievan-ligature-engraver.cc  
b/lily/kievan-ligature-engraver.cc
index  
cde8bf31fbc4ca6312fb35f9eb899fb6f43da019..f7906ffefb66fa13a5e0c54bed8442333b779332  
100644
--- a/lily/kievan-ligature-engraver.cc
+++ b/lily/kievan-ligature-engraver.cc
@@ -40,6 +40,7 @@ protected:
 public:
   TRANSLATOR_DECLARATIONS (Kievan_ligature_engraver);
   TRANSLATOR_INHERIT (Coherent_ligature_engraver);
+  TRANSLATOR_INHERIT (Ligature_engraver);

 private:
   void fold_up_primitives (vector<Grob_info> const &primitives, Real  
padding, Real &min_length);
Index: lily/mensural-ligature-engraver.cc
diff --git a/lily/mensural-ligature-engraver.cc  
b/lily/mensural-ligature-engraver.cc
index  
d122aca7091f40cf84f67c28a169a49647f0412b..34a90c2a65dd5f93b8bbcace92dadacfc0bae06c  
100644
--- a/lily/mensural-ligature-engraver.cc
+++ b/lily/mensural-ligature-engraver.cc
@@ -63,6 +63,7 @@ protected:
 public:
   TRANSLATOR_DECLARATIONS (Mensural_ligature_engraver);
   TRANSLATOR_INHERIT (Coherent_ligature_engraver);
+  TRANSLATOR_INHERIT (Ligature_engraver);

 private:
   void transform_heads (vector<Grob_info> const &primitives);
Index: lily/vaticana-ligature-engraver.cc
diff --git a/lily/vaticana-ligature-engraver.cc  
b/lily/vaticana-ligature-engraver.cc
index  
08d02865f20916c2b0768864f4aadfd276a9caad..38b815411f1aa6901fe9c75c1b71eb89765c5cc8  
100644
--- a/lily/vaticana-ligature-engraver.cc
+++ b/lily/vaticana-ligature-engraver.cc
@@ -78,6 +78,7 @@ private:
 public:
   TRANSLATOR_DECLARATIONS (Vaticana_ligature_engraver);
   TRANSLATOR_INHERIT (Gregorian_ligature_engraver);
+  TRANSLATOR_INHERIT (Ligature_engraver);
 protected:
   virtual Spanner *create_ligature_spanner ();
   virtual void transform_heads (Spanner *ligature,