Re: Tie: add some C++ rigor and concision (issue 252230043 by address@hidden)

[David Kastrup]

David Kastrup <address@hidden> writes:

> address@hidden writes:
>
>> Reviewers: ,
>>
>> Description:
>> I wanted to see how Tie would look with instance methods instead of
>> static methods.  I'm looking forward to hearing your opinions.
>
> Here's a script in progress that currently does way, way too much.

While I am at it, I should likely include the patches leading up to
this.  Commit messages leave something to be desired...

>From 97bf0435e2144e2dca545df8daa654aeee16697a Mon Sep 17 00:00:00 2001
From: David Kastrup <address@hidden>
Date: Sat, 23 May 2015 14:13:40 +0200
Subject: [PATCH 1/2] Make member function versions of MAKE_SCHEME_CALLBACK_*

---
 lily/include/lily-guile-macros.hh | 70 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/lily/include/lily-guile-macros.hh 
b/lily/include/lily-guile-macros.hh
index 869a52a..7329c81 100644
--- a/lily/include/lily-guile-macros.hh
+++ b/lily/include/lily-guile-macros.hh
@@ -237,4 +237,74 @@ inline T *ly_assert_smob (SCM var, int number, const char 
*fun)
 #define LY_ASSERT_SMOB(klass, var, number)                              \
   ly_assert_smob<klass> (var, number, __FUNCTION__)
 
+
+/*
+  Adds the NAME as a Scheme function, and a variable to store the SCM
+  version of the function in the static variable NAME_proc
+*/
+#define DECLARE_MEMBER_CALLBACK(NAME, ARGS)     \
+  SCM NAME ARGS;                                \
+  static SCM NAME ## _proc
+
+string mangle_cxx_identifier (string);
+
+void ly_add_type_predicate (void *ptr, const string &name);
+string predicate_to_typename (void *ptr);
+
+/*
+  Make TYPE::FUNC available as a Scheme function.
+*/
+#define MAKE_MEMBER_CALLBACK_WITH_OPTARGS(TYPE, FUNC, ARGCOUNT, 
OPTIONAL_COUNT, DOC) \
+  SCM TYPE ::FUNC ## _proc;                                             \
+  void                                                                  \
+  TYPE ## _ ## FUNC ## _init_functions ()                               \
+  {                                                                     \
+    string cxx = string (#TYPE) + "::" + string (#FUNC);                \
+    string id = mangle_cxx_identifier (cxx);                            \
+    TYPE ::FUNC ## _proc = scm_c_define_gsubr                           \
+      (id.c_str(),                                                      \
+       (ARGCOUNT-OPTIONAL_COUNT), OPTIONAL_COUNT, 0,                    \
+       (scm_t_subr) checked_trampoline<TYPE, &TYPE::FUNC>);             \
+    ly_add_function_documentation (TYPE :: FUNC ## _proc, id.c_str(), "", \
+                                   DOC);                                \
+    scm_c_export (id.c_str (), NULL);                                   \
+  }                                                                     \
+                                                                        \
+  ADD_SCM_INIT_FUNC (TYPE ## _ ## FUNC ## _callback,                    \
+                     TYPE ## _ ## FUNC ## _init_functions);
+
+#define MAKE_DOCUMENTED_MEMBER_CALLBACK(TYPE, FUNC, ARGCOUNT, DOC)      \
+  MAKE_MEMBER_CALLBACK_WITH_OPTARGS(TYPE, FUNC, ARGCOUNT, 0, DOC);
+
+#define MAKE_MEMBER_CALLBACK(TYPE, FUNC, ARGCOUNT)              \
+  MAKE_MEMBER_CALLBACK_WITH_OPTARGS(TYPE,FUNC,ARGCOUNT, 0, "");
+
+template <class Final, SCM (Final::*fun)()>
+SCM checked_trampoline (SCM self)
+{
+  Final *p = LY_ASSERT_SMOB (Final, self, 1);
+  return (p->*fun) ();
+}
+
+template <class Final, SCM (Final::*fun)(SCM)>
+SCM checked_trampoline (SCM self, SCM arg1)
+{
+  Final *p = LY_ASSERT_SMOB (Final, self, 1);
+  return (p->*fun) (arg1);
+}
+
+template <class Final, SCM (Final::*fun)(SCM, SCM)>
+SCM checked_trampoline (SCM self, SCM arg1, SCM arg2)
+{
+  Final *p = LY_ASSERT_SMOB (Final, self, 1);
+  return (p->*fun) (arg1, arg2);
+}
+
+template <class Final, SCM (Final::*fun)(SCM, SCM, SCM)>
+SCM checked_trampoline (SCM self, SCM arg1, SCM arg2, SCM arg3)
+{
+  Final *p = LY_ASSERT_SMOB (Final, self, 1);
+  return (p->*fun) (arg1, arg2, arg3);
+}
+
 #endif /* LILY_GUILE_MACROS_HH */
-- 
2.1.4

>From 7115a12ef7d663fa93311bbef4fc9dd43b11ef1f Mon Sep 17 00:00:00 2001
From: David Kastrup <address@hidden>
Date: Sat, 23 May 2015 14:47:53 +0200
Subject: [PATCH 2/2] Preedit

---
 lily/relative-octave-music.cc | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lily/relative-octave-music.cc b/lily/relative-octave-music.cc
index 52efa50..4f7efd8 100644
--- a/lily/relative-octave-music.cc
+++ b/lily/relative-octave-music.cc
@@ -30,16 +30,14 @@ public:
 
 MAKE_SCHEME_CALLBACK (Relative_octave_music, no_relative_callback, 2)
 SCM
-Relative_octave_music::no_relative_callback (SCM /* music */,
-                                             SCM pitch)
+Relative_octave_music::no_relative_callback (SCM, SCM pitch)
 {
   return pitch;
 }
 
 MAKE_SCHEME_CALLBACK (Relative_octave_music, relative_callback, 2)
 SCM
-Relative_octave_music::relative_callback (SCM /* music */,
-                                          SCM pitch)
+Relative_octave_music::relative_callback (SCM, SCM pitch)
 {
   return pitch;
 }
-- 
2.1.4

As can be seen from the dates of the commits, it's been some time since
I last worked on this.  At least the first patch in the series should be
helpful as a starting point.  One of the main motivations was that our
callback functions, as a rule, don't do typechecking, leading to lots of
potential for crashes.  By turning the functions into member functions
and letting the corresponding trampolines do the type verification while
establishing the this-pointer, this is addressed summarily rather than
at each call location.

-- 
David Kastrup