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

[Jean Abou Samra]

Le 22/04/2022 à 09:52, Luca Fascione a écrit :
> [snipped]
> Where the problem is that dwc.free(p) is actually effectively acting 
> as if it was dwc2.free(p); because the API doesn't pass around the 
> context like the C++ wrappers appear to do, rather it statefully "just 
> goes for it".
> This is a design decision of Guile, obviously. However, it seems to me 
> this has possibly uncommon semantics even if it were implemented on 
> the scheme-language side of things anyways, doesn't it? I guess I'm 
> asking whether people would want/need to do this on purpose, and why. 
> It seems to me to achieve this behaviour one would have to capture the 
> dwc context and then invoke dwc.free() on that context from deeper 
> inside its own stack, and I'm not clear what this means for the frames 
> intervening inbetween the calling frame and the callee frame, being a 
> current parent of it, maybe it's ok, I normally think of continuations 
> as things to deal with a stack that has been unwound, not deeper parts 
> of a stack that is still being executed.



I guess it would be possible to make it work, using 
scm_current_dynamic_state
and scm_set_current_dynamic_state. I don't have a need to implement it, 
though.
Assumptions should always be introduced just after creating the dynamic 
context.


> [...]
>
> I think the first observation is very good: making it easiest to spot 
> wrong code without paying large amounts of attention has been a very 
> good strategy for me in the past.
> And for the same reason, assuming the scm_dynwind_XXX set of calls is 
> small, I would either wrap all of them (because they're all small and 
> easy) or very clearly document the wrapper class as "if you need this 
> method, add it here and this is how": you definitely don't want to 
> find yourself in a halfway house where there are all sorts of 
> exceptions that "yes these three methods could have been wrapped but 
> we didn't" and now nobody remembers any longer why a decision was 
> taken one way or the other. It's a small cost now and can save many 
> headaches later.
> Obv this works if the set of scm_dynwind_XXX is small (maybe up to 
> 10-15 calls?), if you have several dozens, you'd definitely wrap a few 
> important ones that you need, and leave the rest for another day (with 
> the comment inviting the next person along to help with the effort).



You both made a good point. My conclusion is that I should stick to

  {
    Local_assumption la (Lily::prebreak_estimate, SCM_BOOL_T);
    ...
  }

where the constructor does everything, included scm_dynwind_begin ()
and scm_dynwind_fluid (). That's safer and clearer, and never mind that
you have to construct 3 contexts if you need 3 assumptions. I expect
this to be very rare anyway.



> As to the exchange Jean and I were having about non-local control 
> flow, I was worried about scm_dynwind_end() being called one extra 
> time (rather than not being called enough, as your example 
> demonstrates) and how to detect that, but looking at how 
> scm_dynamic_wind() is implemented, maybe from the C side this can't 
> happen. I'm asking: is there a case where evaluating the "main", 
> non-guard thunk implies a call to scm_dynwind_end(), so that the outer 
> C code doesn't need to? And I think the answer to that is no. So I 
> guess as long as we can guarantee that the think doesn't use call/cc 
> (or that we always use SCM_F_DYNSTACK_FRAME_REWINDABLE) this will be ok?



Re "can scm_dynwind_end () be called unpaired in the thunk": if the callee
respects programming contracts, no. The principle of scm_dynwind_begin 
() and
scm_dynwind_end () is that they should always be paired under normal 
circumstances
(when no Guile non-local control flow happens).

Re call/cc: I would actually like to use 
static_cast<scm_t_dynwind_flags> (0)
and not SCM_F_WIND_EXPLICITLY. I have no idea whether the stuff I write
is going to interact gracefully with rewinding the stack via continuations,
and I honestly don't want to wonder.We're using C++ (on contrary to Guile,
which is primarily tailored for C), so most allocations (e.g., vectors) are
managed via C++ RAII. This means that C++'s stack unwinding and Guile's 
don't
interact, so you can easily crash LilyPond with continuations and such, 
by making
some expression return twice and arranging for it to cause double free. This
is the first example that came to my mind:

\version "2.23.7"

{
  \override NoteHead.X-extent =
    #(let ((cont #f))
       (lambda (grob)
         (if cont
             (cont '(0 . 0))
             (call/cc
              (lambda (c)
                (set! cont c)
                '(0 . 0))))))
  c d
}



Frankly, I don't think we need to bother at all about such stuff. If the 
user
is willing this hard to shoot themselves in the foot, we can't prevent them.

Echoing Han-Wen's fear about the problems linked with dynamic states, 
the reason
to use fluids, apart from consistency with the existing (*parser*) and 
(*location*),
is to be a little cooperative with basic and actually useful non-local flow
such as (mock example, would need to be a little more complex for the 
exceptions
to be actually clearer):

(lambda (grob)
  ;; die if any of the children is dead
  (catch 'child-dead
    (lambda ()
      (under-assumptions ((*prebreak-estimate* #t))
        (reduce interval-union
                empty-interval
                (map (lambda (elt)
                       (if (grob::is-live? elt)
                           (ly:grob-extent grob X)
                           (throw 'child-dead)))
                     (ly:grob-array->list (ly:grob-object grob 
'elements))))))
    (lambda stuff
      (ly:grob-suicide! grob))))


More fancy use of continuations is not something I care about, just like 
nothing
in LilyPond cares about it.


Thanks,
Jean