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

[Jon Ciesla]

On Thu, Mar 3, 2016 at 1:23 AM, <address@hidden> 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
> address@hidden
> 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