Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Hi Luca,

Thanks for your insights! Much appreciated.


Le 21/04/2022 à 08:55, Luca Fascione a écrit :
> I'd think you can up this by one, and get a cleaner looking piece of 
> code if you implement scm_dynwind_fluid() as a forwarded method on 
> your context:
>
> {
>
>   Dynwind_context dwc; // overloaded so you can call 
> dwc(SCM_F_DYNWIND_REWINDABLE);
>   dwc.fluid (fluid1, value1);
>   dwc.fluid (fluid2, value2);
>
>   dwc.unwind_handler(handler, data, flags); // overloaded for the SCM 
> vs void* cases,
>   dwc.rewind_handler(handler, data, flags); // overloaded for the SCM 
> vs void* cases,
>                                             // maybe it ought to check 
> if the constructor was DYNWIND_REWINDABLE and complain
>                                             // (only in debug builds?) 
> if things are not set up right
>   dwc.free(something);
>
> }



Making a method of the context is an attractive proposal, as
it prevents from forgetting to introduce a context.

Actually, the specific use case is much narrower than this particular
set of methods: I don't need C++ wrappers for all dynwind-related Guile
APIs, I just have one need for fluids. The example in the first email was
a straw man for the sake of explanation.


> However in all this, I must say I don't understand this passage in manual:
>
>     The context is ended either implicitly when a non-local exit
>     happens, or explicitly with scm_dynwind_end. You must make sure
>     that a dynwind context is indeed ended properly. If you fail to
>     call scm_dynwind_end for each scm_dynwind_begin, the behavior is
>     undefined.
>
>
> It seems to me the first phrase and the rest slightly contradict each 
> other: as I hear it, the first phrase says "either the C side calls 
> scm_dynwind_end(), OR a non-local exit happens", whereas the rest 
> seems to be saying "the C side _shall_ call scm_dynwind_end". This 
> bothers me, because in the second case our C++ is nice and lovely, but 
> in the first meaning the destructor of dwc has to somehow figure out 
> whether a non-local exit has happened, and avoid calling 
> scm_dynwind_end(). And I don't think scm library can cope on its own, 
> because these things look to me like they would nest, so making 
> scm_dynwind_end() idempotent without some sort of explicit marker on 
> the scope seems... hard.
>
> So yes, I'd think RAII is the idiomatic way to go, I would add the 
> wrappers because they make the pattern cleaner, but do figure out 
> what's up with this last question first, because it could bring it all 
> crumbling down.


Let's do a little test:

diff --git a/lily/main.cc b/lily/main.cc
index c98db72e8c..b8e928e29c 100644
--- a/lily/main.cc
+++ b/lily/main.cc
@@ -496,6 +496,21 @@ do_chroot_jail ()
 }
 #endif

+class Dynwind_context
+{
+public:
+  Dynwind_context ()
+  {
+    message ("starting dynwind context");
+    scm_dynwind_begin (static_cast<scm_t_dynwind_flags> (0));
+  }
+  ~Dynwind_context ()
+  {
+    message ("ending dynwind context");
+    scm_dynwind_end ();
+  }
+};
+
 static void
 main_with_guile (void *, int, char **)
 /*
@@ -505,6 +520,17 @@ main_with_guile (void *, int, char **)
  * to main_with_guile.
  */
 {
+  message ("normal termination:");
+  {
+    Dynwind_context dwc;
+    message ("before normal termination");
+  }
+  message ("termination by exception:");
+  {
+    Dynwind_context dwc;
+    message ("before termination by exception");
+    scm_throw (ly_symbol2scm ("x"), SCM_EOL);
+  }
   /* Engravers use lily.scm contents, so we need to make Guile find it.
      Prepend onto GUILE %load-path.
       %load-path is the symbol Guile searches for .scm files


Running this, I get:

normal termination:
starting dynwind context
before normal termination
ending dynwind context
termination by exception:
starting dynwind context
before termination by exceptionBacktrace:
In ice-9/boot-9.scm:
  1752:10  1 (with-exception-handler _ _ #:unwind? _ # _)
In unknown file:
           0 (apply-smob/0 #<thunk 7f043fd362e0>)

ERROR: In procedure apply-smob/0:
Throw to key `x' with args `()'.

As you can see, scm_dynwind_end () is not called on the non-local exit.

There are actually two kinds of non-local exits that we are talking about
here: the C++ ones, and the Guile ones. C++ does them via return,
break, continue, C++ exceptions. Guile does them via Guile exceptions
and continuations. When Guile captures a continuation, for example,
it just copies the whole C(++) stack in an object, and reinstating
the continuation simply copies the saved stack data back into the
stack. See the end of
https://www.gnu.org/software/guile/manual/html_node/Continuations.html

Exceptions are kind of a special case of this (except that in Guile 2
and later it looks like they are actually implemented with
delimited/composable continuations, aka prompts, rather than traditional
continuations, but I haven't looked at that too closely).

Thus, C++ RAII stuff ensures destructors get called upon C++ non-local
exits, but not Guile ones.

So the meaning of the docs here is that you must insert a call to
scm_dynwind_end () that will work under all circumstances that are
normal from the Guile point of view, including local control flow,
and C++ non-local control flow. Guile arranges to terminate dynamic
contexts in the case of Guile non-local control flow.

The RAII approach allows not to forget calling scm_dynwind_end ()
for C++ non-local control flow.


Cheers,
Jean