Implement suggestions from issue #587 in stepA of several languages

[asarhaddon]

Related with issue #587 .

• Merge eval-ast and eval into a single conditional.

• Print "EVAL: $ast" at the top of EVAL.

• Expand macros during the apply phase, removing lots of duplicate
  tests, and increasing the overall consistency by allowing the macro
  to be computed instead of referenced by name (((defmacro! cond (...))) is currently illegal for example).

Each change was already applied by some implementations.

With the two last changes, macroexpand and quasiquoteexpand special
forms are not needed anymore.

Not tested locally: common-lisp, julia, io, logo, lua, matlab, vhdl,
vimscript

If macroexpand is kept:

• an optional test should be added for (macroexpand ()), which
  currently fails on several implementations,

• it should only apply one macro at a time. The purpose being
  debugging, hiding an iteration does not help. Moreover, the loop is
  currently untested (and probably wrong in nim).

[kanaka]

It's been a few months since I've been able to do anything mal related. After mulling this change for a while, I think I've decided I now like the idea. It does simplify the flow quite a bit and in particular, I think it aligns well with mal's first class macro functions. And I agree with eval_ast not being a fundamental part of the structure. It's okay if implementations take that approach, but I agree it's an implementation detail and probably not the best structure in most cases anyway.

I think the main change I would like to request is that we keep some form of user invokable macro expansion. I think this is an important tool for learning/pedagogy (in addition to debugging). I do agree that it should only expand one level (so renaming it to "macroexpand-1" is probably good). If you can think of a clean/nice way to use your new mechanism/flow then that would be great, otherwise, I'm fine with keeping the separate macroexpand function renamed to macroexpand-1 (and without the loop).

If you're still willing to push this forward (applying the changes to step8 and 9 and fixing conflicts caused earlier merges), then I think this is nearly ready for merging.

We can create a new low-priority ticket with a list of implementations remaining to convert to the new model (if ever).

[asarhaddon]

Please also comment on the changes in process/step*.txt. They are quite intrusive so I do not want them to implicitly pass under your radar.

The macro changes affect steps 8-A, but the eval-ast changes affect steps 2-A.

About macroexpand-1...

In my opinion, it is better that each EVAL outputs its AST, not only macro expansions. The log is noisy, but consistent with the experience in previous steps for new implementors.

A different question is "how should this echo be enabled"?

It is convenient to enable the echo interactively without restarting the interpreter. This is what macroexpand provides, though not for all evaluations. A global state is also possible: (set echo-eval true).
Some implementations already perform macroexpansion inside EVAL.

• ada.2 implements macroexpand with a mutable boolean in the EVAL loop.
• haskell and prolog implement macroexpand as a separate function duplicating EVAL.
  The first solution ensures consistency between macroexpand and actual macro expansion, but would require quite intrusive changes in language with immutable data.
  This added complexity in EVAL, especially with language-specific details, is not worth the while, especially if the motivation is educational.

A good compromise would allow one to enable the echo without editing the source code. Since command line arguments are already mandatory, we could for example let step6 replace the print command (optional and commented in previous steps) with a conditional block triggered by an initial --echo-eval single argument.

Most languages already embed a print command in comments, but this is not convenient, especially with compiled languages. Also, it has already happened that the command is obsolete when one actually requires it.

[kanaka]

I reviewed the process/step*.txt changes and they looked fine at first pass. I think my only minor hesitation is that the line lengths have jumped so things don't look as great in some editor configurations (the previous pseudo-code didn't exceed 80 characters). I'll do more thorough review and maybe even work through a few steps based on the update guide and pseudo-code to make sure things are consistent and easy to follow (I haven't done that in a while so it would be a good refresher).

One thing I realized this weekend is that the diagrams will need to be updated because they refer to eval-ast. I might take the opportunity myself to port the diagrams from gliffy to drawio so they are easier to update with this and future changes.

Configurable debug is a great idea. I had an idea for how to do it:

diff --git a/impls/python/step3_env.py b/impls/python/step3_env.py
index 75ec834e..2e0bfe4d 100644
--- a/impls/python/step3_env.py
+++ b/impls/python/step3_env.py
@@ -10,7 +10,9 @@ def READ(str):
 
 # eval
 def EVAL(ast, env):
-    #print("EVAL %s" % printer._pr_str(ast))
+    edbg = env.get(types._symbol('*DEBUG-EVAL*'))
+    if edbg != None and edbg != False:
+        print("EVAL %s" % printer._pr_str(ast))
 
     if types._symbol_Q(ast):
         return env.get(ast)
@@ -50,6 +52,7 @@ repl_env = Env()
 def REP(str):
     return PRINT(EVAL(READ(str), repl_env))
 
+repl_env.set(types._symbol('*DEBUG-EVAL*'), False)
 repl_env.set(types._symbol('+'), lambda a,b: a+b)
 repl_env.set(types._symbol('-'), lambda a,b: a-b)
 repl_env.set(types._symbol('*'), lambda a,b: a*b)

This enables per lexical scope debug but also global debug if desired:

user> (let* [*DEBUG-EVAL* true] (+ 2 (+ 3 4)))
EVAL (+ 2 (+ 3 4))
EVAL +
...
user> (def! *DEBUG-EVAL* true)
true
user> (+ 2 (+ 3 4))
EVAL (+ 2 (+ 3 4))
EVAL +
...

Obviously the same idea would be propagated forwarded from step3 and described in the process as an optional but useful thing to implement at step3 (with an optional test case to make sure it's doing the right thing). And step6 could support a command line parameter for enabling the global setting by default (again, optional).

One thing I really like about it is that it provides a template for more narrower debug (i.e. implementors could create other *DEBUG-FOO* style environment variables to trigger debug prints or other behavior deeper within other contexts).

While I like the dynamically enabled debug flag idea and think it's definitely worthwhile, I'm not completely convinced though that this fully replaces macroexpand/macroexpand-1. The problem with the eval debug prints is that it happens before evaluation and you don't see the actual results of the eval (you can sometimes infer them based on subsequent things that are printed out) but I've found being able to macroexpand important for debugging in some implementations that I've done. We could have macroexpand be an optional feature in the guide and tests (and suggested as a helpful way to debug quasiquote and macro issues).

[asarhaddon]

Hello.
I have reduced the width of the lines in the process .txt files.
The symbol in MAL environments is a clever solution. I have implemented it in some languages and adapted the process.
I was previously suggesting a command line option, but (def! DEBUG-EVAL true) is both less difficult to implement and more flexible thanks to let*.
I fail to understand what macroexpand would print that DEBUG-EVAL would not. The result of macro expansion is sent to EVAL (or the TCO loop is restarted), so it will be displayed before anything else happens. Unless I am missing something, DEBUG-EVAL prints strictly more information that {macro,quasiquote}expand. Could you provide an example ?
There is little hope to ever bring these changes to low-level languages like nasm and wasm. For nasm, I have managed to adapt stepA, but the steps differ much from each other and backporting is not a grateful job. This is also true for higher level languages, though less annoying. This is a real obstruction, artificially multiplying the cost of maintenance. Why not suggest to new submitters to minimize the diff between steps, from stepA down to step0 ? Another option is to completely remove steps0-9 as soon as the initial porter agrees, or becomes unreachable.

[asarhaddon]

Hello.
A few issues remain before I squash the changes into a single commit.

c.2: the failure on github is unreproducible with gcc 11.12.0.
objpascal: unreproducible with fpc 3.2.2.
ocaml: unreproducible with 4.11.1.
perl6: unreproducible with rakudo 2021.09.
The solution is probably to update the Docker image. I have documented my configuration in #588.

elm: the failure is most probably unrelated because the directory is unchanged.

[asarhaddon]

Hello.
It would be nice to at least update the documentation, so that new implementors spend less time on macros.
The purescript implementation is a good example, and has been created while this request was pending.

[kanaka]

@asarhaddon Sorry for going silent for 2.5 years 😱. I'm starting to work through some of the backlog that has accumulated. This PR is really a significant improvement to the implementations and process. You may have moved on to other things in the meantime, but if you're still getting updates, I wanted to let you know that I've merged this now. I did some tweaks to some of the commits so it doesn't show up as a regular merge in github UI.

Details of what I tweaked:

• removed the wasm changes since that was causing the sum2 test to timeout on large values (I tried 4000 and that works the first few times I run it and then on the 3rd or 4th, it hangs). So possible a memory management issue.
• fixed some implementations that broke with the "a:" symbol test (powershell, prolog, swift5).
• updated Dockerfile for c.2,make,perl6,ocaml,objpascal to get around the errors that appeared with the old compilers.

There are still three failures: elm, common-lisp, and objpascal. The elm issue will probably be addressed by updates you have in a different PR. The objpascal issues looks like a memory management issue (access error when mal exits). I suspect it's been there all along and just doing a better job at detecting this when the process exits. Everything passes except the command line invocations in step6 which require a successfully exit code.

Again, thanks for your excellent work on this and sorry for the REALLY slow response time.