egallesio / STklos

STklos Scheme
http://stklos.net
GNU General Public License v2.0
67 stars 17 forks source link

Simplify error checking #654

Open jpellegrini opened 2 weeks ago

jpellegrini commented 2 weeks ago

@egallesio this is mostly a matter of style. See if you agree...

In the compiler, instead of

  (if some-condition
      ( X ...
          ...
          ... )
      (compiler-error Y))

we do

  (unless some-condition (compiler-error Y))
  ( X ...
      ...
      ... )

Or when, when it makes sense

This makes error checking isolated in the place where it programatically happens, and also reduces source code size a little bit, making it clearer.

There are already uses of the macros when and unless in the compiler, so we're not introducing a dependency on them.

jpellegrini commented 2 weeks ago

I know that there are other places in STklos source that use the if style for error checking; I only changed the compiler, but I could in the future do the same in other files, gradually -- if you believe this is a good change.

egallesio commented 2 weeks ago

Hi @jpellegrini,

The form you propose is not equivalent to the original writing. In fact compiler-error calls error in interactive mode, bit it doesn't when we compile a file (it just prints an error message). For instance, in

(define (compile-quote expr env tail?)
  (unless (= (length expr) 2) (compiler-error 'quote expr "bad usage in ~S" expr))
  (compile-constant (cadr expr) env tail?))

If you call compile-quote with (quote), you'll access the cadr of a list of length 1, which will abort the compilation of the rest of the file.

jpellegrini commented 2 weeks ago

Oh, I saw that the new form wasn't equivalent, but I thought it wouldn't be relevant, because it passed all tests. Maybe we need more tests then! :grin:

egallesio commented 1 week ago

Oh, I saw that the new form wasn't equivalent, but I thought it wouldn't be relevant, because it passed all tests. Maybe we need more tests then! 😁

The problem here is that the tests (in make test) are not compiled, but just loaded.