bug#72372: srfi-64: test-approximate evaluates test-name multiple times

bug#72372: srfi-64: test-approximate evaluates test-name multiple times

[Tomas Volf]

Hello,

I think I found a bug in (srfi srfi-64) module shipped with GNU Guile.

The specification says the following regarding the test-approximate:

> This is equivalent to (except that each argument is only evaluated once):
>
>
> (test-assert [test-name]
>   (and (>= test-expr (- expected error))
>        (<= test-expr (+ expected error))))

However the test-name is evaluated multiple times:

    (use-modules (srfi srfi-64))
    (test-begin "x")
    (test-approximate (pk "t") 1 1 0)

Leading to:

    ;;; ("t")

    ;;; ("t")

Have a nice day
Tomas Volf

bug#72372: srfi-64: test-approximate evaluates test-name multiple times

[Taylan Kammer]

On 30.07.2024 21:51, Tomas Volf wrote:
> Hello,
>
> I think I found a bug in (srfi srfi-64) module shipped with GNU Guile.
>
> The specification says the following regarding the test-approximate:
>
>> This is equivalent to (except that each argument is only evaluated once):
>>
>>
>> (test-assert [test-name]
>>   (and (>= test-expr (- expected error))
>>        (<= test-expr (+ expected error))))
> However the test-name is evaluated multiple times:
>
>     (use-modules (srfi srfi-64))
>     (test-begin "x")
>     (test-approximate (pk "t") 1 1 0)
>
> Leading to:
>
>     ;;; ("t")
>
>     ;;; ("t")
>
> Have a nice day
> Tomas Volf
>
>
Looks like a bug in the reference implementation that Guile uses. It specifically evaluates the macro input `tname` at the beginning, binding it to the variable `name`, but then never uses `name` and just uses `tname` again, leading to it being evaluated twice.

If someone wants to fix it upstream, it should be easy: Just change `tname` to `name` on line 788.

My implementation already seems to have this fixed, because the affected section of the code was entirely rewritten. (In case someone is missing the context: I've explained in a previous email that I don't want to work on the reference implementation / the one shipped with Guile, because I find it too unreadable and difficult to work with, although in this particular case it looks like a trivial bug.)

- Taylan