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