Summary

parameterize bug

Metadata

Description

The following message was sent by Joo ChurlSoo by e-mail:

 The followin seems to be a bug.
 
 CHICKEN
 (c) 2008-2015, The CHICKEN Team
 (c) 2000-2007, Felix L. Winkelmann
 Version 4.10.0 (rev b259631)
 windows-mingw32-x86 [ manyargs dload ptables ]
 compiled 2015-08-04 on yves.more-magic.net (Linux)
 #;1>
 (define a (make-parameter 1 number->string))
 (define b (make-parameter 2 number->string))
 (list (a) (b))
 
 #;2> #;3> ("1" "2")
 #;4> (parameterize ((a 10) (b 20) (a 30)) (list (a) (b)))
 ("30" "20")
 #;5> (list (a) (b))
 ("10" "2")
 #;6> (a 1)
 "1"
 #;7> (list (a) (b))
 ("1" "2")
 #;8> (parameterize ((a 10) (b 20) (a 'abc)) (list (a) (b)))
 
 Error: (number->string) bad argument type: abc
 #;8> (list (a) (b))
 ("10" "20")
 #;9> (a 1) (b 2)
 "1"
 #;10> "2"
 #;11> (parameterize ((a 10) (b 20) (a 30) (a 40) (b 30)) (list (a) (b)))
 ("40" "30")
 #;12> (list (a) (b))
 ("30" "20")

Changes and comments

[2015-12-06 21:14:30 UTC] sjamaan wrote:

This is going to be very tricky to fix in an efficient manner, if you assume any parameterize may fail/throw an exception.

AFAICT, the only truly reliable way to do that is to actually expand it in the "naive" way, exactly as if you had a nested parameterize call for every variable, like so:

 #;1> (define a (make-parameter 1 number->string))
 (define b (make-parameter 2 number->string))
 (list (a) (b)) 
 ("1" "2")
 
 #;2>
 (parameterize ((a 10))
   (parameterize ((b 20))
     (parameterize ((a 'abc))
       (list (a) (b)))))
 Error: (number->string) bad argument type: abc
 #;3> (list (a) (b))
 ("1" "2")

Unfortunately, if we did this, it would mean one dynamic-wind per parameterized variable. Dynamic-wind is pretty inefficient, so if you're setting several variables at once, this will slow things down a lot. Anything else needs to deal with nonlocal exits and of course exception handling.

However, there are still a few improvements we can make!

First, the behaviour of (parameterize ((a 10) (b 20) (a 30)) (list (a) (b))) can be fixed relatively easy by performing the restores of the variables in reverse (or perhaps storing the parameters in a list and performing some sort of deduplication, but that's of course much trickier).

OK, so looking at it again I think it can be fixed after all.

Our main problem is the following:

 #;1> (define a (make-parameter 1 number->string))
 (define b (make-parameter 2 number->string))
 #;2> (list (a) (b))
 ("1" "2")
 (parameterize ((a 10) (b 'abc)) (list (a) (b)))
 Error: (number->string) bad argument type: abc
 #;3> (list (a) (b))
 ("10" "2")

This could in theory be fixed by first calculating all the converted values in a big let and only after they've been converted, set the parameters to the new value. This would require changing the internal "hidden" API that's used. Currently there's no way to extract the converter from a parameter; you can only set the parameter to a value, optionally passing it through the converter.

Currently, the expansion looks something like this:

 #!scm
 (let ((a2 a) (b4 b))
   (let ((g6 10) (g8 'abc))
     (let ((mode #f))
       (let ((swap (lambda ()
                     (let ((t (a2))) (a2 g6 mode) (set! g6 t))
                     ;; If this raises an exception, a2 won't be restored
                     (let ((t (b4))) (b4 g8 mode) (set! g8 t))
                     (set! mode #t))))
         (dynamic-wind swap (lambda () (list (a) (b))) swap)))))

So to make this work, we'd need something like the following (inventing a new parameter API as I go along):

 #!scm
 (let ((a2 a) (b4 b))
   (let ((g6 10) (g8 'abc))
     (let ((mode #f))
       (let ((swap (lambda ()
                     (let ((new-a (if mode g6 (a2 g6 'convert-only)))
                           (new-b (if mode g8 (b4 g8 'convert-only)))
                       ;; Remember current values
                       (set! g6 (a2))
                       (set! g8 (b4))
                       ;; Now we can safely set the values
                       (a2 new-a 'set-only)
                       (b4 new-b 'set-only)
                       (set! mode #t))))
         (dynamic-wind swap (lambda () (list (a) (b))) swap)))))

Here we really should also use two flavours of swap like I described above: the "after" setting the parameters in reverse order, to avoid setting a to the incorrect value if it's parameterized twice in the same expression. If we do this, we also don't need to mess about with the mode variable, which looks a bit iffy to me. As I understand it, mode won't get toggled if something breaks halfway through. On the other hand, if that happens we're screwed anyway, because half the temp variables that are being swapped will have the old values and the other half the new values.

[2015-12-06 21:14:30 UTC] sjamaan wrote:

1449432947278537

[2015-12-06 21:15:59 UTC] sjamaan changed milestone from someday to 4.11.0

[2016-02-09 09:03:09 UTC] evhan changed status from new to closed

[2016-02-09 09:03:09 UTC] evhan set resolution to fixed

[2016-02-09 09:03:09 UTC] evhan wrote:

Fixed by a94b69e and 9a63e35.

[2016-02-09 09:03:09 UTC] evhan wrote:

1455005187261683