Reader macro tests could be more rigourous

[wgmyers]

I had a go at implementing Mal in Ruby and somehow managed to get all the way to completion of Step A - including passing all the self-hosting tests - with a huge bug in the reader macro expansion code.

This only came to light when make "perf^foo" failed.

Turned out that the 'time' function was being truncated after the first let* in the quasiquote. Replacing the backtick with the quasiquote form typed out longhand showed me where the bug was.

I'm struggling to figure out what the best way to test this is because this is also my first exposure to any kind of Lisp, with which I am also struggling tbh, but something like:

;; Reader Macro Expansion Test
(defmacro! expansiontest
  (fn* ()
    (let* [sym (symbol "s")]
      `(let* (~sym ("bol"))
         (do
           (println "sym: " ~sym))))))

would have caught this bug - the above should produce:

sym:  (bol)

but with incorrect quasiquote expansion, gets nothing at all, or just blows up.

[kanaka]

That's a good catch. Can you send a pull request with a test addition to step7 tests that covers this? Then I'll activate Github CI against the PR to make sure that all the other implementations also pass and merge it if so (and if they don't pass then we'll work on fixing those implementations first).

[wgmyers]

Just working on this now, and have hit an 'I don't really know what I am doing' problem.

In order to use the test above, I have put the test addition into step8 rather than step7, so I can use defmacro!.

The test has been modified slightly - the macro is as above but collapsed to one line (test suite doesn't seem to like multi-line expressions); also the spurious space at the end of the first arg to println has been removed, and looks like this:

;; Testing reader macro expansion in complex expressions
(defmacro! expansiontest (fn* () (let* [sym (symbol "s")] `(let* (~sym ("bol")) (do (println "sym:" ~sym))))))
(expansiontest)
;=>sym: (bol)

This passes fine in my own ruby-wgm implementation, but seems to fail for a variety of reasons wherever else I test it (ruby, c, rust, perl, go, python).

This causes me to suspect that my own implementation is doing the wrong thing. If so, I can't work out why.

Bit stumped.

Not sure whether to go ahead and submit a pull request anyway just yet: please advise.

[asarhaddon]

Hi. The ada.2 implementation answers:

user> (defmacro! expansiontest (fn* () (let* [sym (symbol "s")] `(let* (~sym ("bol")) (do (println "sym:" ~sym))))))
user> (expansiontest)
Uncaught exception: first element must be a function or macro
  in eval: ("bol")
  in eval: (let* (s ("bol")) (do (println "sym:" s)))

This makes sense. let* tries to evaluate ("bol") before assigning s, but the list fails to evaluate during the apply phase because its first element is a string, hence not executable.

Could you please submit your interpreter and/or test the following commands?

(macroexpand (expansiontest))
((fn* () (let* [sym (symbol "s")] `(let* (~sym ("bol")) (do (println "sym:" ~sym))))))
(let* [sym (symbol "s")] `(let* (~sym ("bol")) (do (println "sym:" ~sym))))
`(let* (s ("bol")) (do (println "sym:" s)))

All four should answer (let* (s ("bol")) (do (println "sym:" s))).

I am also curious of the answer of step1 to these lines, so that we can see the raw unevaluated effect of the "`" reader macro.

[wgmyers]

Thank you.

My interpreter is here fwiw: https://github.com/wgmyers/mal/tree/ruby-wgm/impls/ruby-wgm - running the four tests you suggested do indeed all answer (let* (s ("bol")) (do (println "sym:" s))).

The underlying issue is me not yet having a full understanding of when lists are evaluated during the apply phase, attempting to use expressions like ("bol") when a correct implementation should complain, and having a buggy implementation which allows such things when it shouldn't.

In the meantime, if I just replace ("bol") with (list "bol") my test should behave as expected. So far it does, both on my implementation and the others I am immediately able to try out.

[asarhaddon]

A bug crashing perf.mal after passing all existing tests is definitely worth a new test. But I am a bit lost in your history. Could you please refer to a commit passing all existing tests and crashing make perf^ruby-wgm (or a more simple valid program, of course)? Thanks.

[wgmyers]

I think the commit that fixed it was this one:

https://github.com/wgmyers/mal/commit/ca801cc1e82926c9a37b52e9dc1b87c094a08d38#diff-1cff364f582518ba9c02dd928b6df7f85e697c5639b59ccf5bedf3cf09968656

[asarhaddon]

It seems difficult to extract a minimal test, and such a test may not be as useful as I expected because few implementations will mix list/vector/map delimitors and reader macros.