let-values can bind variables incorrectly

[yorickhardy]

When I tried the ray tracer https://git.sr.ht/~jakob/lisp-raytracer-zoo/tree/master/item/r7rs-raytracer.scm with cyclone, I see the error

Error: Invalid type: #(<Record Marker> <plane> #(#(<Record Marker> <vec3> #(0.0 -1.25 0.0)) #(<Record Marker> <vec3> #(0.0 1.0 0.0)) #(<Record Marker> <material> #(#(<Record Marker> <vec3> #(1.0 1.0 0.2)) #(<Record Marker> <vec3> #(1.0 1.0 0.2)) () () () () ())))) expected <vec3>
Call history, most recent first:
[1] scheme/base.sld:raise
[2] scheme/base.sld:error
[3] scheme/base.sld:call-with-values
[4] scheme/base.sld:values
[5] scheme/base.sld:call-with-values
[6] r7rs-raytracer.scm:reduce
[7] scheme/base.sld:zero?
[8] r7rs-raytracer.scm:vec3-
[9] r7rs-raytracer.scm:spot-light-at

Patching r7rs-raytracer.scm with (i.e. replacing let-values) yields a working ray tracer:

--- r7rs-raytracer.scm  2023-07-16 17:26:59.686717278 +0200
+++ r7rs-raytracer2.scm 2023-07-16 18:34:08.540508821 +0200
@@ -479,13 +479,17 @@
     (let loop2 ((y 0))
       (unless (>= y image-height)
         (vector-set! image (coordinate->index x y)
-                     (let-values (((x y) (screen->viewport x y)))
-                       (let* ((ray (coordinate->ray x y))
-                              (intersection (ray-intersect-scene ray)))
-                         (if (is-some? intersection)
-                             (let-values (((shape position) (apply values (unwrap intersection))))
-                               (vec3->color (shade-pixel shape position (ray-origin ray))))
-                             (ray-color (coordinate->ray x y))))))
+                     (call-with-values
+                       (lambda () (screen->viewport x y))
+                       (lambda (x y)
+                         (let* ((ray (coordinate->ray x y))
+                                (intersection (ray-intersect-scene ray)))
+                           (if (is-some? intersection)
+                               (apply
+                                 (lambda (shape position)
+                                  (vec3->color (shade-pixel shape position (ray-origin ray))) )
+                                 (unwrap intersection) )
+                               (ray-color (coordinate->ray x y)))))))
         (loop2 (+ y 1))))
     (unless (>= x (- image-width 1))
       (loop1 (+ x 1))))

The problem seems to be that (let-values (((shape position) (...))) ...) binds the same value to shape and position (both are bound to a shape). I tested this a bit more in the ray tracer code and the value bound to the first variable is bound also to all subsequent variables -- but I cannot reproduce it yet in a small example (small examples all seemed to work as expected). I also looked at the implementation of let-values and could not find any bugs, sorry!

[yorickhardy]

After macro expansion, the following code segment is generated:

 (call-with-values
   (lambda () (screen->viewport x$1215 y$1219))
   (lambda (x$1215$1225 x$1215$1226)
     ((lambda (x$1228 y$1229)
        ((lambda ()
           ((lambda (ray$1232)
              ((lambda (intersection$1235)
                 ((lambda ()
                    (if (is-some?
                          intersection$1235)
                      (call-with-values
                        (lambda ()
                          (apply values
                                 (unwrap
                                   intersection$1235)))
                        (lambda (x$1228$1242
                                 x$1228$1243)
                          ((lambda (shape$1245
                                    position$1246)
                             ((lambda ()
                                (vec3->color
                                  (shade-pixel
                                    shape$1245
                                    position$1246
                                    (ray-origin
                                      ray$1232))))))
                           x$1228$1242
                           x$1228$1242)))
                      (ray-color
                        (coordinate->ray
                          x$1228
                          y$1229))))))
               (ray-intersect-scene ray$1232)))
            (coordinate->ray x$1228 y$1229)))))
      x$1215$1225
      x$1215$1225))))

In the inner call-with-values, the last argument should be x$1228$1243, while the last argument for the outer call-with-values should be x$1215$1226

[yorickhardy]

Still investigating: if I change one of the identifiers in the templates in the syntax-rules for let-values, then it seems to work.

--- scheme/base.sld.orig    2023-03-04 18:54:17.000000000 +0000
+++ scheme/base.sld
@@ -1950,9 +1950,9 @@
        "mktmp"
        b
        e0
-       (arg ... x)
+       (arg ... xyzzy)
        bindings
-       (tmp ... (a x))
+       (tmp ... (a xyzzy))
        body))
     ((let-values
        "mktmp"

[yorickhardy]

The renaming of variables in the previous comment obviously does not work in general. The problem is that free variables "x" are introduced in the template, but they may already be bound in the scope surrounding the macro evaluation. Here is a small example:

(import (scheme base) (scheme write))

; x is free in a template in scheme/base.sld let-values
; expected output (1 2)
; actual output   (1 1)
(let ( (x 3)  )
 (let-values ( ((x y) (values 1 2)) )
  (display (list x y)) (newline) ) )

The easiest fix is perhaps to just set let-values == let*-values.

[yorickhardy]

Apologies for the many replies!

This is probably issue #431 and not really about let-values.

[justinethier]

No need for apologies @yorickhardy - thanks for raising this issue and working through it!