[bug#76485] gexp: ‘with-parameters’ properly handles ‘%graft?’.
Commit Message
Fixes <https://issues.guix.gnu.org/75879>.
* guix/gexp.scm (mcall-with-parameters): New procedure.
(compile-parameterized): Use it instead of ‘with-fluids’.
* tests/gexp.scm ("with-parameters for %graft?"): New test.
Reported-by: David Elsing <david.elsing@posteo.net>
Change-Id: Iddda7ead2aeef24dd989ac37a53fc99b726731b3
---
guix/gexp.scm | 31 ++++++++++++++++++++++++-------
tests/gexp.scm | 20 ++++++++++++++++++++
2 files changed, 44 insertions(+), 7 deletions(-)
base-commit: 90aa90eb05429553402e0b5225d23f84742a9286
Comments
Hello,
Ludovic Courtès <ludo@gnu.org> writes:
> + (mcall-with-parameters
> + parameters
> + (map (lambda (thunk) (thunk)) values)
Composing the monadic value here is a good idea I think.
> + ;; XXX: Non-local exits can leave PARAMETERS set to VALUES.
> + (mlet* %store-monad ((old-values
> + (return (map set-value parameters values)))
> + (result (thunk)))
> + (mbegin %store-monad
> + (return (map set-value parameters old-values)) ;restore old values
> + (return result))))
However, I'm not convinced it is meaningful to set the parameters for a
general monad, e.g. for the identity monad or the list monad, there is
no way for the parameters to have an effect, only during the evaluation
of a function. Instead, I would suggest to only do this for the state
monad, as the parameters have an effect during the function application
of the monadic value to the state. The same applies to `mparameterized'
in (guix monads). Do you think that makes sense?
This allows for the use of `with-fluids*', keeping the parameterization
local. In the following message is a patch with the changes, which
(apart from the issue below) appears to work.
When testing the patch however, I noticed that `with-fluids*' does not
work with prompts and I get the following error:
--8<---------------cut here---------------start------------->8---
ERROR: Wrong type (expecting resumable continuation): #<vm-continuation 7f595e325690>
--8<---------------cut here---------------end--------------->8---
Attached is a minimal example, which works fine if `with-fluids*' is
removed. Is this behavior expected (I didn't see it in the
documentation) or is it a bug in Guile?
Cheers,
David
(define p (make-parameter 1))
(define cont
(call-with-prompt 'foo
(lambda ()
(with-fluids* (list (parameter-fluid p))
(list 2)
(lambda ()
(+ (abort-to-prompt 'foo) (p)))))
(lambda (k)
k)))
(p 3)
(pk (cont 0))
Hi David,
David Elsing <david.elsing@posteo.net> skribis:
>> + ;; XXX: Non-local exits can leave PARAMETERS set to VALUES.
>> + (mlet* %store-monad ((old-values
>> + (return (map set-value parameters values)))
>> + (result (thunk)))
>> + (mbegin %store-monad
>> + (return (map set-value parameters old-values)) ;restore old values
>> + (return result))))
>
> However, I'm not convinced it is meaningful to set the parameters for a
> general monad, e.g. for the identity monad or the list monad, there is
> no way for the parameters to have an effect, only during the evaluation
> of a function.
Still, ‘mparameterized’ does the right thing, whether you’re using
‘%state-monad’ or ‘%identity-monad’:
--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,optimize (mparameterize %identity-monad ((current-output-port (%make-void-port "w")))
(mlet* %identity-monad ((x (return 2))
(y (return 3))
(z (return (+ x y))))
(return (* z 2))))
$26 = (let ((old-value (current-output-port)))
(current-output-port (%make-void-port "w"))
(let ((mvalue 10)) (current-output-port old-value) mvalue))
scheme@(guile-user)> ,optimize (mparameterize %state-monad ((current-output-port (%make-void-port "w")))
(mlet* %state-monad ((x (return 2))
(y (return 3))
(z (return (+ x y))))
(return (* z 2))))
$27 = (let* ((old-value (current-output-port))
(mvalue
(let ((value (current-output-port (%make-void-port "w"))))
(lambda (state) (values value state)))))
(lambda (state)
(call-with-values
(lambda () (mvalue state))
(lambda (value state)
((begin
(current-output-port old-value)
(lambda (state) (values 10 state)))
state)))))
--8<---------------cut here---------------end--------------->8---
That is, no matter what, it changes the value of the parameters before
evaluation of the body happens (whether or not it’s delayed) and changes
it back after.
> Instead, I would suggest to only do this for the state monad, as the
> parameters have an effect during the function application of the
> monadic value to the state. The same applies to `mparameterized' in
> (guix monads). Do you think that makes sense?
Yes, though it would be nice to have a variant of ‘parameterize’ “that
works for monad”, that was the intent of ‘mparameterize’.
Because of that, I have a preference for ‘mparameterize’ rather than
‘state-parameterize’ or any other specific variant.
WDYT?
> This allows for the use of `with-fluids*', keeping the parameterization
> local. In the following message is a patch with the changes, which
> (apart from the issue below) appears to work.
Yes, that’s because ‘with-fluids*’ is implemented in C, which makes it a
“continuation barrier” (continuations that contain C stack frames cannot
be resumed). It’s a limitation of the current implementation rather
than a bug, strictly speaking. :-)
Thanks,
Ludo’.
Ludovic Courtès <ludo@gnu.org> skribis:
> Yes, though it would be nice to have a variant of ‘parameterize’ “that
> works for monad”, that was the intent of ‘mparameterize’.
I meant: “that works for any monad”.
> Because of that, I have a preference for ‘mparameterize’ rather than
> ‘state-parameterize’ or any other specific variant.
Ludo’.
Hi Ludo',
I thought about it some more and I think my issue is that in general,
the number of times a function passed to bind is evaluated is not fixed.
Attached is an example for a monad which acts as a combination of the
state monad and the maybe monad. I used two parameters, because they are
set at different times in the implementation of `mparameterize' (but
this seems to be a different issue to me). In particular, p2 is not
restored at the end, because no function application is done for a
"nothing" value.
Another problematic example would be a "coroutine" monad [1], which can
pause the evaluation of its "steps", during which the parameters could
be used or changed elsewhere.
Ludovic Courtès <ludo@gnu.org> writes:
> Yes, though it would be nice to have a variant of ‘parameterize’ “that
> works for any monad”, that was the intent of ‘mparameterize’.
I think what you describe (although I'm not too familiar with Haskell)
is like the ReaderT (or StateT if the parameters can change within the
monad) monad transformer, where the parameters are stored in the
additional state provided by ReaderT. This is however a new monad and
different from setting the parameters globally in different functions
passed to bind.
> Because of that, I have a preference for ‘mparameterize’ rather than
> ‘state-parameterize’ or any other specific variant.
>
> WDYT?
To my understanding, what we actually want is to affect the way the
function of the state monad applies to the passed state, or formulated
in a different way, the state to include the parameters. This would be
effectively achieved using `with-fluids*' inside the monadic procedure
(except that they are not part of the final state). It can be though of
expanding the state of the state monad with the parameters, where the
initial state contains the "outside" parameters. Of course, now there
are two different ways to pass on state, through the state monad and the
parameters... :)
Does this make sense?
Because `with-fluids*' does not work with prompts, I still think your
solution is a good workaround when specialized for the state monad, as
long as the parameters are not used externally until the execution is
completely finished.
> Yes, that’s because ‘with-fluids*’ is implemented in C, which makes it a
> “continuation barrier” (continuations that contain C stack frames cannot
> be resumed). It’s a limitation of the current implementation rather
> than a bug, strictly speaking. :-)
Ah I see, thanks for the explanation! Are there plans to change that and
do you think it would be difficult to do?
Maybe the suspension could also be done without prompts by instead
modifying the store monad, similar to [1]? It would probably be less
straightforward though and maybe better suitable for languages like
Haskell.
Cheers,
David
[1] https://hackage.haskell.org/package/monad-coroutine-0.9.2/docs/Control-Monad-Coroutine.html
(use-modules
(guix monads)
(ice-9 match))
(define-inlinable (test-return value)
(lambda (state)
(list 'just value state)))
(define-inlinable (test-bind mvalue mproc)
(lambda (state)
(match (mvalue state)
(('just value state2)
((mproc value) state2))
(('nothing state2)
(list 'nothing state2)))))
(define-monad %test-monad
(bind test-bind)
(return test-return))
(define mval
(mlet %test-monad
((a (return 3))
(b (lambda (state) (list 'nothing state))))
(return 2)))
(define p1 (make-parameter 1))
(define p2 (make-parameter 1))
(pk "A" (p1) (p2))
(define mval2
(mparameterize %test-monad
((p1 3)
(p2 3))
mval))
(pk "B" (p1) (p2))
(p1 2)
(p2 2)
(pk "C" (p1) (p2))
(pk (mval2 1))
(pk "D" (p1) (p2))
;;; Output:
;;; ("A" 1 1)
;;; ("B" 3 1)
;;; ("C" 2 2)
;;; ((nothing 1))
;;; ("D" 2 3)
Hi David,
David Elsing <david.elsing@posteo.net> skribis:
> I thought about it some more and I think my issue is that in general,
> the number of times a function passed to bind is evaluated is not fixed.
> Attached is an example for a monad which acts as a combination of the
> state monad and the maybe monad. I used two parameters, because they are
> set at different times in the implementation of `mparameterize' (but
> this seems to be a different issue to me). In particular, p2 is not
> restored at the end, because no function application is done for a
> "nothing" value.
>
> Another problematic example would be a "coroutine" monad [1], which can
> pause the evaluation of its "steps", during which the parameters could
> be used or changed elsewhere.
Hmm right.
I guess my contradiction is that I’m looking for a “generic” solution
but whose genericity is limited by the set of monads defined in (guix
monads) and by my imagination. :-)
>> Because of that, I have a preference for ‘mparameterize’ rather than
>> ‘state-parameterize’ or any other specific variant.
>>
>> WDYT?
>
> To my understanding, what we actually want is to affect the way the
> function of the state monad applies to the passed state, or formulated
> in a different way, the state to include the parameters. This would be
> effectively achieved using `with-fluids*' inside the monadic procedure
> (except that they are not part of the final state). It can be though of
> expanding the state of the state monad with the parameters, where the
> initial state contains the "outside" parameters. Of course, now there
> are two different ways to pass on state, through the state monad and the
> parameters... :)
>
> Does this make sense?
I think so. :-)
The core of the problem here is that (guix monads), to a large extent,
addresses problems already addressed by other Scheme constructs such as
parameters/fluids, but in an incompatible way. So really,
‘mparameterize’ and the many commits that fixed interactions between
‘%current-system’ & co. and the monad are really band aid.
So in this case I’m also looking for a “quick fix” more than extending
(guix monads).
> Because `with-fluids*' does not work with prompts, I still think your
> solution is a good workaround when specialized for the state monad, as
> long as the parameters are not used externally until the execution is
> completely finished.
Yes, the semantics are clumsy.
Would you like to send a patch that does it the way you want?
Preferably limited to fixing ‘with-parameters’ in particular so it also
works for ‘%graft?’.
Or are you saying that the patch at the beginning of this thread (where
‘mcall-with-parameters’ is specialized for the state monad) is good
enough?
Tell me what you prefer. :-)
(There’s a couple of patch series depending on this fix.)
Thanks,
Ludo’.
Hi Ludo',
Ludovic Courtès <ludo@gnu.org> writes:
> I guess my contradiction is that I’m looking for a “generic” solution
> but whose genericity is limited by the set of monads defined in (guix
> monads) and by my imagination. :-)
Ah yes, but I don't think a generic solution is needed at this point, as
only the state monad is used in Guix. :)
I also tried to find a general way to apply `with-fluids*' for monads in
`mparameterize' and then noticed it might not make much sense.
> The core of the problem here is that (guix monads), to a large extent,
> addresses problems already addressed by other Scheme constructs such as
> parameters/fluids, but in an incompatible way. So really,
> ‘mparameterize’ and the many commits that fixed interactions between
> ‘%current-system’ & co. and the monad are really band aid.
I don't find the combination of the state monad and parameters so bad,
and the idea of using the store monad to accumulate what to build is
quite nice. To me the combination with prompts is a bit confusing
though, it might be clearer to have continuations as part of the store
monad.
> So in this case I’m also looking for a “quick fix” more than extending
> (guix monads).
Yes that makes sense, in my opinion a quick fix for the state monad is
fine.
> Would you like to send a patch that does it the way you want?
> Preferably limited to fixing ‘with-parameters’ in particular so it also
> works for ‘%graft?’.
>
> Or are you saying that the patch at the beginning of this thread (where
> ‘mcall-with-parameters’ is specialized for the state monad) is good
> enough?
Oh, maybe you didn't see my previous patch where I specialized
`mcall-with-parameters' and `mparameterize' to the state monad, I only
sent it to Debbugs. I updated the patch and used your workaround for
avoiding using `with-fluids*' by setting and later restoring the
parameters.
Cheers,
David
Hey David,
David Elsing <david.elsing@posteo.net> skribis:
> Ah yes, but I don't think a generic solution is needed at this point, as
> only the state monad is used in Guix. :)
> I also tried to find a general way to apply `with-fluids*' for monads in
> `mparameterize' and then noticed it might not make much sense.
Yeah, I agree.
>> The core of the problem here is that (guix monads), to a large extent,
>> addresses problems already addressed by other Scheme constructs such as
>> parameters/fluids, but in an incompatible way. So really,
>> ‘mparameterize’ and the many commits that fixed interactions between
>> ‘%current-system’ & co. and the monad are really band aid.
>
> I don't find the combination of the state monad and parameters so bad,
> and the idea of using the store monad to accumulate what to build is
> quite nice.
That’s right, glad you like it. :-)
Overall I think the initial motivation for having monads (the “store”
monad in particular) still makes sense. It’s the integration that
turned out to be clumsier than expected.
> To me the combination with prompts is a bit confusing though, it might
> be clearer to have continuations as part of the store monad.
I’m not sure. In a way, file-like objects achieve something similar to
the store monad, but in a more “Schemey” way. Likewise, Scheme has
delimited continuations, which is dynamic in nature; in other languages
one would use a monad for that but in Scheme it would be questionable as
it wouldn’t play well with other features (first of all by
“contaminating” non-monadic procedures).
> Oh, maybe you didn't see my previous patch where I specialized
> `mcall-with-parameters' and `mparameterize' to the state monad, I only
> sent it to Debbugs. I updated the patch and used your workaround for
> avoiding using `with-fluids*' by setting and later restoring the
> parameters.
I actually did see it but I was in a state of confusion. :-)
Thanks for your patience and for your work!
Ludo’.
@@ -728,19 +728,34 @@ (define-syntax-rule (with-parameters ((param value) ...) body ...)
(lambda ()
body ...)))
+(define (mcall-with-parameters parameters values thunk)
+ "Set PARAMETERS to VALUES for the dynamic extent of THUNK, a monadic
+procedure."
+ ;; This is the procedural variant of 'mparameterize'.
+ (define (set-value parameter value)
+ (parameter value))
+
+ ;; XXX: Non-local exits can leave PARAMETERS set to VALUES.
+ (mlet* %store-monad ((old-values
+ (return (map set-value parameters values)))
+ (result (thunk)))
+ (mbegin %store-monad
+ (return (map set-value parameters old-values)) ;restore old values
+ (return result))))
+
(define-gexp-compiler compile-parameterized <parameterized>
compiler =>
(lambda (parameterized system target)
(match (parameterized-bindings parameterized)
(((parameters values) ...)
- (let ((fluids (map parameter-fluid parameters))
- (thunk (parameterized-thunk parameterized)))
+ (let ((thunk (parameterized-thunk parameterized)))
;; Install the PARAMETERS for the dynamic extent of THUNK.
- (with-fluids* fluids
- (map (lambda (thunk) (thunk)) values)
+ ;; Special-case '%current-system' and '%current-target-system' to
+ ;; make sure we get the desired effect.
+ (mcall-with-parameters
+ parameters
+ (map (lambda (thunk) (thunk)) values)
(lambda ()
- ;; Special-case '%current-system' and '%current-target-system' to
- ;; make sure we get the desired effect.
(let ((system (if (memq %current-system parameters)
(%current-system)
system))
@@ -2350,4 +2365,6 @@ (define* (references-file item #:optional (name "references")
(read-hash-extend #\$ read-ungexp)
(read-hash-extend #\+ (cut read-ungexp <> <> #t)))
-;;; gexp.scm ends here
+;;; Local Variables:
+;;; eval: (put 'mcall-with-parameters 'scheme-indent-function 2)
+;;; End:
@@ -451,6 +451,26 @@ (define %extension-package
(return (string=? (derivation-file-name drv)
(derivation-file-name result)))))
+(test-assertm "with-parameters for %graft?"
+ (mlet* %store-monad ((replacement -> (package
+ (inherit %bootstrap-guile)
+ (name (string-upcase
+ (package-name
+ %bootstrap-guile)))))
+ (guile -> (package
+ (inherit %bootstrap-guile)
+ (replacement replacement)))
+ (drv0 (package->derivation %bootstrap-guile))
+ (drv1 (package->derivation replacement))
+ (obj0 -> (with-parameters ((%graft? #f))
+ guile))
+ (obj1 -> (with-parameters ((%graft? #t))
+ guile))
+ (result0 (lower-object obj0))
+ (result1 (lower-object obj1)))
+ (return (and (eq? drv0 result0)
+ (eq? drv1 result1)))))
+
(test-assert "with-parameters + file-append"
(let* ((system (match (%current-system)
("aarch64-linux" "x86_64-linux")