Re: Start up GUILE type system before anything else (issue 234260043 by address@hidden)

[dak]

Reviewers: lemzwerg, carl.d.sorensen_gmail.com,

Message:
On 2015/05/06 14:39:02, Carl wrote:
> LGTM.  I'm glad you have enough knowledge of how the compiler works to
make this
> patch!

Actually, the new code stays away as far as possible from the stuff
requiring intimate knowledge of the compiler.

I put in a comment for the _old_ startup code documenting the language
quirks it accommodates, and it took me several days of debugging and
standard reading to understand why the code is written in the manner it
is.

So the part requiring knowledge of how the compiler works is the comment
on the old code.  And the new code stays the heck away from that area.
Maybe it would be worthwhile to move the old code to the kind of scheme
the new one uses.  But at least with the comment in it it's educational.

Description:
Start up GUILE type system before anything else

This permits using our datatypes in the general Scheme initialization
phase without crashing or order dependencies.

This should be a more reliable and less invasive implementation of the
task attempted with issue 4360.

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

Affected files (+31, -6 lines):
  M lily/guile-init.cc
  M lily/include/smobs.hh
  M lily/include/smobs.tcc
  M lily/smobs.cc


Index: lily/guile-init.cc
diff --git a/lily/guile-init.cc b/lily/guile-init.cc
index  
67ed72822ace306853e6b9ac6dd33a3867c1dc9a..86a998e9ab5c71ab23fda0465067c33650ebf8b8  
100644
--- a/lily/guile-init.cc
+++ b/lily/guile-init.cc
@@ -21,11 +21,21 @@
 #include "lily-guile.hh"
 #include "main.hh"
 #include "warn.hh"
+#include "smobs.hh"

 /*
   INIT
 */

+// Why a pointer here?  Because it has zero initialization at load
+// time which is guaranteed to come before the static initializations
+// of all constructors for static expressions of the classes created
+// by ADD_SCM_INIT_FUNC.  The vector data type does not have load-time
+// initialization and might clear out already set callbacks at the
+// time it is initialized since there is no implied order among
+// non-trivial constructors for static data in separate compilation
+// units.  So we need a trivial type like a pointer instead.
+
 typedef void (*Void_fptr) ();
 vector<Void_fptr> *scm_init_funcs_;

@@ -40,6 +50,8 @@ void add_scm_init_func (void (*f) ())
 void
 ly_init_ly_module (void *)
 {
+  // Start up type system first.
+  Scm_init::init ();
   for (vsize i = scm_init_funcs_->size (); i--;)
     (scm_init_funcs_->at (i)) ();

Index: lily/include/smobs.hh
diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh
index  
3ee2ef80abce09b44b2acadf1081173eb6579ce6..2787750ec5b4b58e7393ba2ddbc9fd1df7e61307  
100644
--- a/lily/include/smobs.hh
+++ b/lily/include/smobs.hh
@@ -134,12 +134,15 @@
 // class) to make sure the variable is actually instantiated.

 class Scm_init {
+  static const Scm_init * list_;
+  void (*const fun_)(void);
+  Scm_init const * const next_;
+  Scm_init ();          // don't use default constructor, don't define
+  Scm_init (const Scm_init &);  // don't define copy constructor
 public:
-  Scm_init () { }
-  Scm_init (void (*fun) (void))
-  {
-    add_scm_init_func (fun);
-  }
+  Scm_init (void (*fun) (void)) : fun_ (fun), next_ (list_)
+  { list_ = this; }
+  static void init ();
 };

 template <class Super>
Index: lily/include/smobs.tcc
diff --git a/lily/include/smobs.tcc b/lily/include/smobs.tcc
index  
b4dacacaa325ec8759bf321e368b4c8e138cc0ff..913785315ae9bb46f9d72f1ea6f44233ac5e277e  
100644
--- a/lily/include/smobs.tcc
+++ b/lily/include/smobs.tcc
@@ -110,7 +110,7 @@ template <class Super>
 scm_t_bits Smob_base<Super>::smob_tag_ = 0;

 template <class Super>
-Scm_init Smob_base<Super>::scm_init_ = init;
+Scm_init Smob_base<Super>::scm_init_ (init);

 template <class Super>
 string Smob_base<Super>::smob_name_;
Index: lily/smobs.cc
diff --git a/lily/smobs.cc b/lily/smobs.cc
index  
8b3c8a90141e63a61991dd86579d40ab987dcf59..d36c9be7dc6f6ec560839c6c0b1c8c64c5941739  
100644
--- a/lily/smobs.cc
+++ b/lily/smobs.cc
@@ -85,3 +85,13 @@ unprotect_smob (SCM smob, SCM *prot_cons)
   *prot_cons = SCM_EOL;
 #endif
 }
+
+
+Scm_init const *Scm_init::list_ = 0;
+
+void
+Scm_init::init ()
+{
+  for (Scm_init const *p = list_; p; p = p->next_)
+    p->fun_ ();
+}