lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Remove Smob::type_p_name_ default (issue 287350043 by address@hidden


From: dak
Subject: Re: Remove Smob::type_p_name_ default (issue 287350043 by address@hidden)
Date: Thu, 03 Mar 2016 07:23:33 +0000

Reviewers: carl.d.sorensen_gmail.com,

Message:
On 2016/03/03 00:48:04, Carl wrote:
LGTM.

Thanks for dealing with this so quickly!

Carl

Quickly?  We went months without release because of C++ compiler
problems related to this exact type match already once.  And a number of
patches.  I don't want to restart this cycle of standard/compiler
compliance hunting.

This is not "dealing with this so quickly" rather than "throwing in the
towel right away this time round".

Exactly because I've had it with dealing with it.  And it's actually an
embarrassingly small towel anyway.  Not that much effort to throw it.

Sorry for the months of pain it caused Phil last time round until
Masamichi Hosoda-san stepped up and saved the day.  And stayed around,
so at least something good came of it.

Description:
Remove Smob::type_p_name_ default

This was the single most problematic thing across C++ compilers and
standards.
Foregoing it means a hassle, but using it turned out to be worse.

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

Affected files (+26, -12 lines):
  M lily/all-font-metrics.cc
  M lily/include/all-font-metrics.hh
  M lily/include/listener.hh
  M lily/include/paper-outputter.hh
  M lily/include/scale.hh
  M lily/include/scm-hash.hh
  M lily/include/smobs.hh
  M lily/include/translator-dispatch-list.hh
  M lily/listener.cc
  M lily/paper-outputter.cc
  M lily/scale.cc
  M lily/scm-hash.cc
  M lily/translator-dispatch-list.cc
  M lily/unpure-pure-container.cc


Index: lily/all-font-metrics.cc
diff --git a/lily/all-font-metrics.cc b/lily/all-font-metrics.cc
index ab4f2a4ce4e3a09651b674a8f9598e2cdd6699f5..a560a25940b5dbd065d50ed6ad8d81f6af6b6782 100644
--- a/lily/all-font-metrics.cc
+++ b/lily/all-font-metrics.cc
@@ -27,6 +27,8 @@
 #include "scm-hash.hh"
 #include "warn.hh"

+const char * const All_font_metrics::type_p_name_ = 0;
+
 Index_to_charcode_map const *
 All_font_metrics::get_index_to_charcode_map (const string &filename,
                                              int face_index,
Index: lily/include/all-font-metrics.hh
diff --git a/lily/include/all-font-metrics.hh b/lily/include/all-font-metrics.hh index a2d090a6dfd71c7567447e5ff75dd87ece76f756..f206a3b95d7ec375f6a864c44a8aab7b43170e05 100644
--- a/lily/include/all-font-metrics.hh
+++ b/lily/include/all-font-metrics.hh
@@ -48,6 +48,7 @@ class All_font_metrics : public Smob<All_font_metrics>

   All_font_metrics (All_font_metrics const &);
 public:
+  static const char * const type_p_name_; // = 0
   SCM mark_smob () const;

Index_to_charcode_map const *get_index_to_charcode_map (const string &filename,
Index: lily/include/listener.hh
diff --git a/lily/include/listener.hh b/lily/include/listener.hh
index 00a64eebb892db03b4231cb983e413c9a4051d34..27e7d85b719d70eb8f8d5aa101f60720ea3baeb5 100644
--- a/lily/include/listener.hh
+++ b/lily/include/listener.hh
@@ -179,6 +179,7 @@ class Callback_wrapper : public Simple_smob<Callback_wrapper> Callback_wrapper (void (*trampoline) (SCM, SCM)) : trampoline_ (trampoline)
   { } // Private constructor, use only in make_smob
 public:
+  static const char * const type_p_name_; // = 0
   LY_DECLARE_SMOB_PROC (&Callback_wrapper::call, 2, 0, 0)
   SCM call (SCM target, SCM ev)
   {
Index: lily/include/paper-outputter.hh
diff --git a/lily/include/paper-outputter.hh b/lily/include/paper-outputter.hh index cd1a5e2c92fb4ffb34a0581f85398750c1276d28..8683b4c6d10dd5f04531e40691c3bad00228890f 100644
--- a/lily/include/paper-outputter.hh
+++ b/lily/include/paper-outputter.hh
@@ -33,6 +33,7 @@
 class Paper_outputter : public Smob<Paper_outputter>
 {
 public:
+  static const char * const type_p_name_; // = 0
   SCM mark_smob () const;
   virtual ~Paper_outputter ();
 private:
Index: lily/include/scale.hh
diff --git a/lily/include/scale.hh b/lily/include/scale.hh
index 19cd175c797fd7d9e63a5ac79dbb8c73696c1fa0..7c990e503419ce7db183489fae7ce90c0074d092 100644
--- a/lily/include/scale.hh
+++ b/lily/include/scale.hh
@@ -26,6 +26,7 @@

 struct Scale : public Smob<Scale>
 {
+  static const char * const type_p_name_; // = 0
   virtual ~Scale ();
   Scale (vector<Rational> const &);
   Scale (Scale const &);
Index: lily/include/scm-hash.hh
diff --git a/lily/include/scm-hash.hh b/lily/include/scm-hash.hh
index 3453904f7152cf3c8cfe60b652fca9892803637a..241f316c4ec3dd622ed493936a5dcf63b36f3447 100644
--- a/lily/include/scm-hash.hh
+++ b/lily/include/scm-hash.hh
@@ -46,6 +46,7 @@
 class Scheme_hash_table : public Smob1<Scheme_hash_table>
 {
 public:
+  static const char * const type_p_name_; // = 0
   int print_smob (SCM, scm_print_state *) const;
   bool try_retrieve (SCM key, SCM *val);
   bool contains (SCM key) const;
Index: lily/include/smobs.hh
diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh
index 9ccaa61345565a02a4a50d5ac6d402389993b934..889d86a8cab3a342b374593a7a6afa1879d98f9f 100644
--- a/lily/include/smobs.hh
+++ b/lily/include/smobs.hh
@@ -185,18 +185,11 @@ private:
   static int print_trampoline (SCM, SCM, scm_print_state *);
   static void smob_proc_init (scm_t_bits) { };

-  // type_p_name_ can be overriden in the Super class with a static
-  // const char [] string.  This requires both a declaration in the
-  // class as well as a single instantiation outside.  Using a
-  // template specialization for supplying a different string name
-  // right in Smob_base<Super> itself seems tempting, but the C++
-  // rules would then require a specialization declaration at the
-  // class definition site as well as a specialization instantiation
-  // in a single compilation unit.  That requires just as much source
-  // code maintenance while being harder to understand and quite
-  // trickier in its failure symptoms when things go wrong.  So we
-  // just use a static zero as "not here" indication.
-  static const int type_p_name_ = 0;
+  // type_p_name_ has to be defined in the Super class, either with a
+  // static const char [] string or as a null pointer of type const
+  // char *.  We used to provide a default here for convenience, but
+  // battling the various conflicting C++ standards was too much of a
+  // hassle.

   // LY_DECLARE_SMOB_PROC is used in the Super class definition for
   // making a smob callable like a function.  Its first argument is a
Index: lily/include/translator-dispatch-list.hh
diff --git a/lily/include/translator-dispatch-list.hh b/lily/include/translator-dispatch-list.hh index 6c365d088effb2540559b99b2445849cc952e7b8..6fd03beecf050ce363338430753fb2a22991ac10 100644
--- a/lily/include/translator-dispatch-list.hh
+++ b/lily/include/translator-dispatch-list.hh
@@ -35,6 +35,7 @@ class Engraver_dispatch_list : public Simple_smob<Engraver_dispatch_list>
 {
   vector<Engraver_dispatch_entry> dispatch_entries_;
 public:
+  static const char * const type_p_name_; // = 0
   void apply (Grob_info);
   SCM static create (SCM trans_list,
                      SCM iface_list, Direction);
Index: lily/listener.cc
diff --git a/lily/listener.cc b/lily/listener.cc
index 2a8d28d8cbb6daa0d287eeccded86624f75fbb9d..c1e0b442e25dff095b30944d380adc7d3134ad89 100644
--- a/lily/listener.cc
+++ b/lily/listener.cc
@@ -19,4 +19,6 @@

 #include "listener.hh"

+const char * const Callback_wrapper::type_p_name_ = 0;
+
 const char Listener::type_p_name_[] = "ly:listener?";
Index: lily/paper-outputter.cc
diff --git a/lily/paper-outputter.cc b/lily/paper-outputter.cc
index a72ea59ccdef770d8f70c32f942a42ee4fe8a6d9..471a36a41a4572eb1941eb83ad4a27b244cb3c26 100644
--- a/lily/paper-outputter.cc
+++ b/lily/paper-outputter.cc
@@ -40,6 +40,8 @@ using namespace std;
 #include "lily-imports.hh"


+const char * const Paper_outputter::type_p_name_ = 0;
+
 Paper_outputter::Paper_outputter (SCM port, const string &format)
 {
   file_ = port;
Index: lily/scale.cc
diff --git a/lily/scale.cc b/lily/scale.cc
index 02c1dc5f8e2daca09b480ac977049c66e70d68e3..d6f566b9ed57517d28ae1ef2859d25438deff275 100644
--- a/lily/scale.cc
+++ b/lily/scale.cc
@@ -90,6 +90,8 @@ LY_DEFINE (ly_set_default_scale, "ly:set-default-scale",
   return SCM_UNSPECIFIED;
 }

+const char * const Scale::type_p_name_ = 0;
+
 int
 Scale::step_count () const
 {
Index: lily/scm-hash.cc
diff --git a/lily/scm-hash.cc b/lily/scm-hash.cc
index dd8f9257b933a93d9bfb92b7593180aa3cb16f7d..3eb40ada7bfe929576fc50cb1c081d368951b488 100644
--- a/lily/scm-hash.cc
+++ b/lily/scm-hash.cc
@@ -21,6 +21,8 @@

 #include <cassert>

+const char * const Scheme_hash_table::type_p_name_ = 0;
+
 SCM
 Scheme_hash_table::make_smob ()
 {
Index: lily/translator-dispatch-list.cc
diff --git a/lily/translator-dispatch-list.cc b/lily/translator-dispatch-list.cc index 10bf06407598f452fffffd5b034caa42f7a2f7dc..41d5a171827ceb5bdf065d3eaf5eeebd0bcf1884 100644
--- a/lily/translator-dispatch-list.cc
+++ b/lily/translator-dispatch-list.cc
@@ -21,6 +21,8 @@
 #include "engraver.hh"


+const char * const Engraver_dispatch_list::type_p_name_ = 0;
+
 void
 Engraver_dispatch_list::apply (Grob_info gi)
 {
Index: lily/unpure-pure-container.cc
diff --git a/lily/unpure-pure-container.cc b/lily/unpure-pure-container.cc
index 0e389d4bcd087bf291413155e3398436393b9a17..7a7d6d48d16d3c139aca38ad9f0f38f307b5debb 100644
--- a/lily/unpure-pure-container.cc
+++ b/lily/unpure-pure-container.cc
@@ -25,6 +25,7 @@
 class Unpure_pure_call : public Smob1<Unpure_pure_call>
 {
 public:
+  static const char * const type_p_name_; // = 0
   // Smob procedures unfortunately can only take at most 3 SCM
   // arguments.  Otherwise we could use a "3, 0, 1" call signature and
   // not require an argument count check of our own.
@@ -37,6 +38,8 @@ public:
   }
 };

+const char * const Unpure_pure_call::type_p_name_ = 0;
+
 SCM
 Unpure_pure_container::pure_part () const
 {





reply via email to

[Prev in Thread] Current Thread [Next in Thread]