From: David Kastrup <dak@gnu.org>
Date: Sun, 1 Jan 2017 09:14:14 +0000 (+0100)
Subject: Issue 5024: Rework the Preinit framework into something simpler
X-Git-Url: https://git.donarmstrong.com/lilypond.git?a=commitdiff_plain;h=6786ba7b5cd73f94eec0a49fd68d0e6d9d283437;p=lilypond.git

Issue 5024: Rework the Preinit framework into something simpler

The previous introduction of the Preinit class had to work with
uninitialized data members in a state before construction.

Particularly for data structures like vector classes, this
was quite awkward.  Moving such structures into a separate
class initialized before the smobifying base class fixes
this deficiency and makes the behavior much more reliable,
at the cost of making the class hierarchy a bit more nested.
---

diff --git a/lily/engraver-group.cc b/lily/engraver-group.cc
index 51af2d4afa..c431d10ae1 100644
--- a/lily/engraver-group.cc
+++ b/lily/engraver-group.cc
@@ -204,8 +204,7 @@ Engraver_group::do_announces ()
   while (pending_grobs ());
 }
 
-void
-Engraver_group::pre_init ()
+Preinit_Engraver_group::Preinit_Engraver_group ()
 {
   acknowledge_hash_table_drul_.set (SCM_EOL, SCM_EOL);
 }
diff --git a/lily/global-context.cc b/lily/global-context.cc
index 47e76718db..be523665b9 100644
--- a/lily/global-context.cc
+++ b/lily/global-context.cc
@@ -30,8 +30,7 @@ using namespace std;
 #include "output-def.hh"
 #include "warn.hh"
 
-void
-Global_context::pre_init ()
+Preinit_Global_context::Preinit_Global_context ()
 {
   output_def_ = 0;
 }
diff --git a/lily/include/engraver-group.hh b/lily/include/engraver-group.hh
index 367b6a8f4c..6bdb914314 100644
--- a/lily/include/engraver-group.hh
+++ b/lily/include/engraver-group.hh
@@ -33,16 +33,20 @@ public:
   Direction start_end () const { return start_end_; }
 };
 
-class Engraver_group : public Preinit<Engraver_group>, public Translator_group
+struct Preinit_Engraver_group
+{
+  Drul_array<SCM> acknowledge_hash_table_drul_;
+  Preinit_Engraver_group ();
+};
+
+class Engraver_group : Preinit_Engraver_group, public Translator_group
 {
 protected:
   vector<Announce_grob_info> announce_infos_;
-  Drul_array<SCM> acknowledge_hash_table_drul_;
   void override (SCM);
   void revert (SCM);
 public:
   DECLARE_CLASSNAME (Engraver_group);
-  void pre_init ();
   Engraver_group ();
   virtual void derived_mark () const;
   void do_announces ();
diff --git a/lily/include/global-context.hh b/lily/include/global-context.hh
index fd6f1f92c0..e0aca0a7d6 100644
--- a/lily/include/global-context.hh
+++ b/lily/include/global-context.hh
@@ -23,17 +23,21 @@
 #include "context.hh"
 #include "pqueue.hh"
 
-class Global_context : public Preinit<Global_context>, public Context
+struct Preinit_Global_context
 {
-  PQueue<Moment> extra_mom_pq_;
   Output_def *output_def_;
+  Preinit_Global_context ();
+};
+
+class Global_context : Preinit_Global_context, public Context
+{
+  PQueue<Moment> extra_mom_pq_;
   virtual void derived_mark () const;
 
   DECLARE_CLASSNAME (Global_context);
 
   friend class Output_def;
 public:
-  void pre_init ();
   Global_context (Output_def *);
   int get_moments_left () const;
   Moment sneaky_insert_extra_moment (Moment);
diff --git a/lily/include/modified-font-metric.hh b/lily/include/modified-font-metric.hh
index 66996eaf06..0f4223d4ba 100644
--- a/lily/include/modified-font-metric.hh
+++ b/lily/include/modified-font-metric.hh
@@ -22,8 +22,14 @@
 
 #include "font-metric.hh"
 
+struct Preinit_Modified_font_metric
+{
+  Font_metric *orig_;
+  Preinit_Modified_font_metric ();
+};
+
 /* Perhaps junk this, and move this to layout_def as interface? */
-struct Modified_font_metric : public Preinit<Modified_font_metric>,
+class Modified_font_metric : Preinit_Modified_font_metric,
                               public Font_metric
 {
 public:
@@ -37,10 +43,8 @@ public:
   size_t name_to_index (string) const;
   size_t index_to_charcode (size_t) const;
   Font_metric *original_font () const;
-  void pre_init ();
 
 protected:
-  Font_metric *orig_;
   Real magnification_;
 
   Modified_font_metric (Font_metric *fm, Real magnification);
diff --git a/lily/include/music.hh b/lily/include/music.hh
index 32ca55babc..af672a38d1 100644
--- a/lily/include/music.hh
+++ b/lily/include/music.hh
@@ -27,10 +27,15 @@
 
 #define is_mus_type(x) internal_is_music_type (ly_symbol2scm (x))
 
-class Music : public Preinit<Music>, public Prob
+struct Preinit_Music {
+  SCM length_callback_;
+  SCM start_callback_;
+  Preinit_Music ();
+};
+
+class Music : Preinit_Music, public Prob
 {
 public:
-  void pre_init ();
   Music (SCM init);
   Music (Music const &m);
   VIRTUAL_COPY_CONSTRUCTOR (Music, Music);
@@ -62,8 +67,6 @@ protected:
   virtual void type_check_assignment (SCM, SCM) const;
   virtual void derived_mark () const;
 protected:
-  SCM length_callback_;
-  SCM start_callback_;
   friend SCM ly_extended_make_music (SCM, SCM);
 };
 
diff --git a/lily/include/open-type-font.hh b/lily/include/open-type-font.hh
index bf66d66981..76d0153255 100644
--- a/lily/include/open-type-font.hh
+++ b/lily/include/open-type-font.hh
@@ -26,23 +26,25 @@ Index_to_charcode_map make_index_to_charcode_map (FT_Face face);
 void get_unicode_name (char *s, FT_ULong code);
 void get_glyph_index_name (char *s, FT_ULong code);
 
-class Open_type_font : public Preinit<Open_type_font>, public Font_metric
-{
-  /* handle to face object */
-  FT_Face face_;
-  string postscript_name_;
-
+struct Preinit_Open_type_font {
   SCM lily_subfonts_;
   SCM lily_character_table_;
   SCM lily_global_table_;
   SCM lily_index_to_bbox_table_;
+  Preinit_Open_type_font ();
+};
+
+class Open_type_font : Preinit_Open_type_font, public Font_metric
+{
+  /* handle to face object */
+  FT_Face face_;
+  string postscript_name_;
 
   Index_to_charcode_map index_to_charcode_map_;
   Open_type_font (FT_Face);
 
   DECLARE_CLASSNAME (Open_type_font);
 public:
-  void pre_init ();
   Real get_units_per_EM () const;
   SCM get_subfonts () const;
   SCM get_global_table () const;
diff --git a/lily/include/pango-font.hh b/lily/include/pango-font.hh
index 2a3367705b..aaae077b76 100644
--- a/lily/include/pango-font.hh
+++ b/lily/include/pango-font.hh
@@ -29,19 +29,22 @@
 
 #include "font-metric.hh"
 
-class Pango_font : public Preinit<Pango_font>, public Font_metric
+struct Preinit_Pango_font {
+  SCM physical_font_tab_;
+  Preinit_Pango_font ();
+};
+
+class Pango_font : Preinit_Pango_font, public Font_metric
 {
   PangoContext *context_;
   PangoFontDescription *pango_description_;
   PangoAttrList *attribute_list_;
   Real scale_;
   Real output_scale_;
-  SCM physical_font_tab_;
   Direction text_direction_;
 
 public:
   SCM physical_font_tab () const;
-  void pre_init ();
   Pango_font (PangoFT2FontMap *,
               PangoFontDescription const *,
               Real);
diff --git a/lily/include/scheme-engraver.hh b/lily/include/scheme-engraver.hh
index 2eb2f30379..11caf9f9c2 100644
--- a/lily/include/scheme-engraver.hh
+++ b/lily/include/scheme-engraver.hh
@@ -24,11 +24,23 @@
 
 #include "engraver.hh"
 
-class Scheme_engraver : public Preinit<Scheme_engraver>, public Engraver
+struct Preinit_Scheme_engraver {
+  SCM initialize_function_;
+  SCM finalize_function_;
+  SCM precomputable_methods_ [TRANSLATOR_METHOD_PRECOMPUTE_COUNT];
+
+  // hashq table of interface-symbol -> scheme-function
+  Drul_array<SCM> interface_acknowledger_hash_;
+
+  // Alist of listened-symbol . scheme-function
+  SCM per_instance_listeners_;
+  Preinit_Scheme_engraver ();
+};
+
+class Scheme_engraver : Preinit_Scheme_engraver, public Engraver
 {
 public:
   TRANSLATOR_FAMILY_DECLARATIONS (Scheme_engraver);
-  void pre_init ();
   Scheme_engraver (SCM definition);
 
 protected:
@@ -53,16 +65,6 @@ private:
   SCM translator_description () const { return SCM_EOL; }
 
   bool must_be_last_;
-
-  SCM initialize_function_;
-  SCM finalize_function_;
-  SCM precomputable_methods_ [TRANSLATOR_METHOD_PRECOMPUTE_COUNT];
-
-  // hashq table of interface-symbol -> scheme-function
-  Drul_array<SCM> interface_acknowledger_hash_;
-
-  // Alist of listened-symbol . scheme-function
-  SCM per_instance_listeners_;
 };
 
 #endif /* SCHEME_ENGRAVER_HH */
diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh
index 0d446102e1..00217866dc 100644
--- a/lily/include/smobs.hh
+++ b/lily/include/smobs.hh
@@ -95,6 +95,30 @@
 
   Complex smobs are created by deriving from Smob<Classname>.
 
+  However, this is not sufficient when classes with their own
+  protectable elements are derived from the Complex base class.  This
+  is because initialization order is a tricky thing: once a base class
+  calls smobify_self () in its constructor, further allocations during
+  construction of base class and derived classes might lead to
+  mark_smob calls on the object under construction.  When those call a
+  virtual function like derived_mark, the virtual function
+  corresponding to the incompletely initialized object of derived
+  class type is likely to be called.
+
+  The order of initialization of an object consists in calling the
+  constructors of virtual base classes, then of non-virtual base
+  classes, then initializing all data members.
+
+  As a result, the constructor of a derived class comes too late for
+  initialization of data members that may be accessed in the
+  derived_mark kind of functions.
+
+  Such data members are consequently moved into Preinit_* classes
+  which come before the smobifying base class in derivation order and
+  construct the contained data members in a state suitable for
+  derived_mark calls.
+
+
   CALLING INTERFACE
 
   Common global functions for accessing C++ smob objects:
@@ -320,32 +344,6 @@ public:
   }
 };
 
-// This is a tricky thing: once a base class calls smobify_self () in
-// its constructor, further allocations during construction of base
-// class and derived classes might lead to mark_smob calls on the
-// object under construction.  When those call a virtual function like
-// derived_mark, the virtual function corresponding to the
-// incompletely initialized object is likely to be called.
-//
-// The order of initialization of an object consists in calling the
-// constructors of virtual base classes, then of non-virtual base
-// classes, then initializing all data members.
-//
-// As a result, the derived constructor comes too late for
-// initialization.  That's where the Preinit template class comes in.
-// Derive from it _before_ deriving from the smobifying base class
-// providing derived_mark, and it will call its Base class' pre_init
-// function (which must not rely on the instantiation being complete).
-
-template <class Base>
-class Preinit {
-protected:
-  Preinit ()
-  {
-    (static_cast <Base *> (this)) -> pre_init ();
-  }
-};
-
 extern bool parsed_objects_should_be_dead;
 class parsed_dead
 {
diff --git a/lily/include/spanner.hh b/lily/include/spanner.hh
index bc41ec6e63..a5ecd3dae9 100644
--- a/lily/include/spanner.hh
+++ b/lily/include/spanner.hh
@@ -39,9 +39,16 @@
     is absolutely necessary for beams, since they have to adjust the
     length of stems of notes they encompass.
 */
-class Spanner : public Preinit<Spanner>, public Grob
+
+struct Preinit_Spanner
 {
   Drul_array<Item *> spanned_drul_;
+  SCM pure_property_cache_;
+  Preinit_Spanner ();
+};
+
+class Spanner : Preinit_Spanner, public Grob
+{
   vsize break_index_;
 
   DECLARE_CLASSNAME (Spanner);
@@ -66,7 +73,6 @@ public:
   void set_bound (Direction d, Grob *);
   Item *get_bound (Direction d) const;
 
-  void pre_init ();
   Spanner (SCM);
   Spanner (Spanner const &);
   bool is_broken () const;
@@ -83,8 +89,6 @@ public:
   void cache_pure_property (SCM sym, int start, int end, SCM value);
 
 protected:
-  SCM pure_property_cache_;
-
   void set_my_columns ();
   virtual Grob *clone () const;
   virtual void do_break_processing ();
diff --git a/lily/modified-font-metric.cc b/lily/modified-font-metric.cc
index a93b3f4d97..d68db92c4b 100644
--- a/lily/modified-font-metric.cc
+++ b/lily/modified-font-metric.cc
@@ -26,7 +26,7 @@ using namespace std;
 #include "main.hh"
 #include "program-option.hh"
 
-void Modified_font_metric::pre_init ()
+Preinit_Modified_font_metric::Preinit_Modified_font_metric ()
 {
   orig_ = 0;
 }
diff --git a/lily/music.cc b/lily/music.cc
index 994d685ed2..8b9da5596d 100644
--- a/lily/music.cc
+++ b/lily/music.cc
@@ -45,8 +45,7 @@ Music::internal_is_music_type (SCM k) const
   return scm_is_true (scm_c_memq (k, ifs));
 }
 
-void
-Music::pre_init ()
+Preinit_Music::Preinit_Music ()
 {
   length_callback_ = SCM_EOL;
   start_callback_ = SCM_EOL;
diff --git a/lily/open-type-font.cc b/lily/open-type-font.cc
index 73ef069c4f..730d6cef35 100644
--- a/lily/open-type-font.cc
+++ b/lily/open-type-font.cc
@@ -251,8 +251,7 @@ Open_type_font::make_otf (const string &str)
   return otf->self_scm ();
 }
 
-void
-Open_type_font::pre_init ()
+Preinit_Open_type_font::Preinit_Open_type_font ()
 {
   lily_character_table_ = SCM_EOL;
   lily_global_table_ = SCM_EOL;
diff --git a/lily/pango-font.cc b/lily/pango-font.cc
index 798d258338..9fe6b78cb0 100644
--- a/lily/pango-font.cc
+++ b/lily/pango-font.cc
@@ -46,8 +46,7 @@
 #if HAVE_PANGO_FT2
 #include "stencil.hh"
 
-void
-Pango_font::pre_init ()
+Preinit_Pango_font::Preinit_Pango_font ()
 {
   physical_font_tab_ = SCM_EOL;
 }
diff --git a/lily/scheme-engraver.cc b/lily/scheme-engraver.cc
index 54d9620822..d0abfb1bd0 100644
--- a/lily/scheme-engraver.cc
+++ b/lily/scheme-engraver.cc
@@ -27,15 +27,13 @@
 
 #include "scm-hash.hh"
 
-void
-Scheme_engraver::pre_init ()
+Preinit_Scheme_engraver::Preinit_Scheme_engraver ()
 {
   initialize_function_ = SCM_EOL;
   finalize_function_ = SCM_EOL;
 
   interface_acknowledger_hash_.set (SCM_EOL, SCM_EOL);
 
-  must_be_last_ = false;
   per_instance_listeners_ = SCM_EOL;
   for (int i = 0; i < TRANSLATOR_METHOD_PRECOMPUTE_COUNT; i++)
     precomputable_methods_[i] = SCM_UNDEFINED;
diff --git a/lily/spanner.cc b/lily/spanner.cc
index 1b6798086a..f3593cbbf5 100644
--- a/lily/spanner.cc
+++ b/lily/spanner.cc
@@ -224,13 +224,8 @@ Spanner::set_bound (Direction d, Grob *s)
     Pointer_group_interface::add_grob (i, ly_symbol2scm ("bounded-by-me"), this);
 }
 
-void
-Spanner::pre_init ()
+Preinit_Spanner::Preinit_Spanner ()
 {
-  break_index_ = (vsize)-1;
-  // This is stupid, but derived_mark may be run before broken_into_
-  // has run its constructor and has a recognizable array size.
-  // So we use break_index_ == -1 as a sentinel.
   spanned_drul_.set (0, 0);
   pure_property_cache_ = SCM_UNDEFINED;
 }