From 5a584a973b632f28edcc0312d92e4a38ee567d8c Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Tue, 15 Jun 2021 07:47:29 -0700 Subject: 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. --- eval.c | 41 +++++++++++++++++++++++++++++------------ tests/011/macros-3.tl | 8 ++++++++ txr.1 | 43 +++++++++++++++++++++++++++++-------------- 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 -- cgit v1.2.3