Remove Smob::type_p_name_ default

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.

[Carl, 2016-03-03 00:48:04]

LGTM.

Thanks for dealing with this so quickly!

Carl

[dak, 2016-03-03 07:23:33]

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.

[limburgher_gmail.com, 2016-03-03 14:19:12]

On Thu, Mar 3, 2016 at 1:23 AM, <dak@gnu.org> wrote:

> 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/
>
>

Thank you, I can confirm that this builds with GCC6.


> 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
>  {
>
>
>
> _______________________________________________
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/lilypond-devel
>



-- 
http://cecinestpasunefromage.wordpress.com/
------------------------------------------------
in your fear, seek only peace
in your fear, seek only love

-d. bowie

[c_sorensen, 2016-03-03 19:58:59]

On 3/3/16 12:23 AM, "dak@gnu.org" <dak@gnu.org> wrote:

>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".

I've seen you work long enough to know how much it pains you to have to do
something expedient, rather than right.

I appreciate your willingness to find an acceptable expedient solution,
even though it's not perfect.

Thanks again,

Carl

[dak, 2016-03-03 22:12:52]

Carl Sorensen <c_sorensen@byu.edu> writes:

> On 3/3/16 12:23 AM, "dak@gnu.org" <dak@gnu.org> wrote:
>
>>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".
>
> I've seen you work long enough to know how much it pains you to have
> to do something expedient, rather than right.

It's not wrong, just boorish.  But you don't carry a laser sword into
mud wrestling.  You don't take the chance of running against the
standard committees.  You grit your teeth and walk.

> I appreciate your willingness to find an acceptable expedient
> solution, even though it's not perfect.

It's adequate.  Adequate is good, very good, very excellent good; and
yet it is not; it is but adequate.

-- 
David Kastrup