Re: [PATCH] * src/eval.c: Stop checking for nvars, and use only CONSP

[Stefan Monnier <monnier@iro.umontreal.ca>]

> I found src/eval.c (let) has redundant conditions, that compares
> the length of the list with the current index and also checks if
> the current list is cons.

The patch looks fine, thank you.

Just one detail about the tests: the change you made affects the
evaluation of `let` in the case where the code is interpreted but it
is not used when the code is byte-compiled.  So you might like your
tests to use things like

    (eval '(let ...) t)

to avoid the compiler getting in the way.

> +  ;; (error "`let' bindings can have only one value-form" a 1 2)
> +  (should-error (let ((a 1 2))
> +                  a)
> +                :type 'error)

Also, while this `let` is indeed invalid code, I don't think we
guarantee that it will signal an error, and especially not at runtime
(it's more likely to signal an error at macroexpansion or compile
time).

I think the compiler (or `macroexpand-all`) should make an effort to
detect and diagnose those problems, but I don't think it's important to
catch those problems in the interpreter.


        Stefan