defsymacro: regression: don't expand replacement.

This is a regression that was introduced in 191. The change in
191 was trying to prevent defsymacro from being expanded
immediately by the expander except in 190 compatibility.
Unfortunately, this caused the whole defsymacro block not to
be entered unless in 190 compatibility, otherwise taking the
common exit which returns form_ex, containing the expanded
replacement form.

* eval.c (do_expand): Split up implementation of defvarl and
defsymacro.  In the defsymacro block, do not do any expanding
on entry. Absent of compatibility mode, we just do some sanity
checks and pass the entire form through. In 262 compatibility,
we do the expansion to obtain form_ex. Then all the previous
compat logic is wrapped in that block.

* tests/011/macros-3.tl: Add a test case which confirms that
symbol macros are lazily expanded. Weakness in the test suite
is how these regressions creep in.

* txr.1: Improve defsymacro documentation, spelling out
clearly that the unexpanded replacement form is associated
with the symbol. Eliminate obsolescent text suggesting that
defsymacro is evaluated at macro time.

3 files changed, 66 insertions, 26 deletions

diff --git a/eval.c b/eval.c
index 2ad82dec..50d20fad 100644
--- a/eval.c
+++ b/eval.c
@@ -4749,32 +4749,49 @@ again:
       if (pairs == pairs_ex)
         return form;
       return rlcp(cons(cond_s, pairs_ex), form);
-    } else if (sym == defvarl_s || sym == defsymacro_s) {
+    } else if (sym == defvarl_s) {
       val name = second(form);
       val init = third(form);
       val init_ex = expand(init, menv);
       val form_ex = form;
 
-      if (sym == defsymacro_s && length(form) != three)
-        eval_error(form, lit("~s: two arguments expected"), sym, nao);
-
       if (!bindable(name))
         not_bindable_error(form, name);
 
-      if (sym == defvarl_s)
-        uw_register_tentative_def(cons(var_s, name));
+      uw_register_tentative_def(cons(var_s, name));
 
       if (init != init_ex)
         form_ex = rlcp(cons(sym, cons(name, cons(init_ex, nil))), form);
 
-      if (opt_compat && opt_compat <= 190 && sym == defsymacro_s) {
-        val result = eval(if3(opt_compat && opt_compat <= 137,
-                              form_ex, form),
-                          make_env(nil, nil, nil), form);
-        return cons(quote_s, cons(result, nil));
+      return form_ex;
+    } else if (sym == defsymacro_s) {
+      val name = second(form);
+      val init = third(form);
+
+      if (length(form) != three)
+        eval_error(form, lit("~s: two arguments expected"), sym, nao);
+
+      if (!bindable(name))
+        not_bindable_error(form, name);
+
+      if (opt_compat && opt_compat <= 262) {
+        val init_ex = expand(init, menv);
+        val form_ex = form;
+
+        if (init != init_ex)
+          form_ex = rlcp(cons(sym, cons(name, cons(init_ex, nil))), form);
+
+        if (opt_compat <= 190 && sym == defsymacro_s) {
+          val result = eval(if3(opt_compat && opt_compat <= 137,
+                                form_ex, form),
+                            make_env(nil, nil, nil), form);
+          return cons(quote_s, cons(result, nil));
+        }
+
+        return form_ex;
       }
 
-      return form_ex;
+      return form;
     } else if (sym == lambda_s) {
       if (!cdr(form))
         eval_error(form, lit("~s: missing argument list"), sym, nao);
diff --git a/tests/011/macros-3.tl b/tests/011/macros-3.tl
index bf7cf9a6..e08e5da7 100644
--- a/tests/011/macros-3.tl
+++ b/tests/011/macros-3.tl
@@ -10,3 +10,11 @@
           (macrolet ((m (:form f) f))
             (m))))))
   42)
+
+(defvar x 0)
+(defmacro mac-time-counter () (inc x))
+(defsymacro s (mac-time-counter))
+
+(mtest s 1
+       s 2
+       s 3)
diff --git a/txr.1 b/txr.1
index 4a3b4309..7c7554fb 100644
--- a/txr.1
+++ b/txr.1
@@ -37464,15 +37464,30 @@ between a symbol
 .meta sym
 and and a
 .metn form .
-The binding denotes the form itself, rather than its value. How the
-symbol macro works is that if
+The binding denotes the form itself, rather than its value.
+
+The
+.meta form
+argument is not subject to macro expansion; it is associated with
 .meta sym
-occurs as a form in a scope where the symbol macro definition is
-in scope,
+in its unexpanded state, as it appears in the
+.code defmacro
+form.
+
+The
+.code defsymacro
+form must be evaluated for its defining to to take place; therefore,
+the definition is not available in the top-level form which contains the
+.code defsymacro
+invocation; it becomes available to a subsequent top-level form.
+
+Subsequent to the evaluation of the
+.code defsymacro
+definition, whenever the macro expander encounters
 .meta sym
-is replaced by
+sym as a form, it replaces it by
 .metn form .
-Immediately after this replacement takes place,
+After this replacement takes place,
 .meta form
 itself is then processed for further replacement of macros and
 symbol macros.
@@ -37485,14 +37500,6 @@ like
 .code set
 and similar.
 
-A
-.code defsymacro
-form is implicitly executed at expansion time, and thus need
-not be wrapped in a
-.code macro-time
-form, just like
-.codn defmacro .
-
 Note: if a symbol macro expands to itself directly, expansion stops. However,
 if a symbol macro expands to itself through a chain of expansions,
 runaway expansion-time recursion will occur.
@@ -81870,6 +81877,14 @@ of these version values, the described behaviors are provided if
 is given an argument which is equal or lower. For instance
 .code "-C 103"
 selects the behaviors described below for version 105, but not those for 102.
+.IP 262
+Selection 262 compatibility restores a wrong behavior which existed between
+versions 191 and 262 due to a regression. The wrong behavior is that the
+.code defsymacro
+operator macro-expanded the replacement form, instead of associating the
+macro symbol with the unexpanded form. This makes a crucial difference
+to symbol macros which rely on expansion-time effects, such as producing a
+different expansion each time they are used.
 .IP 258
 Selecting 258 or lower compatibility causes
 .code abs-path-p