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,
