[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
{