bug#14422: 24.3; Eager Macro Expansion

[Achim Gratz]

In GNU Emacs 24.3.1 (i686-suse-linux-gnu, GTK+ Version 3.8.1) of 2013-04-27
Windowing system distributor `The X.Org Foundation', version 11.0.11302000
System Description: openSUSE 12.3/Tumbleweed (i586)

The following test case demonstrates a problem that has been distilled
from Org's test suite. Org has since switched to use defun instead of
defmacro to work around this issue, but it seems that this might be a
corner case that eager macro expansion produces or not yet warn about
(whatever the intended behaviour might be).

[Stefan Monnier]

> The following test case demonstrates a problem that has been distilled
> from Org's test suite.

% emacs24 -Q --batch -f batch-byte-compile eme.el

In toplevel form:
eme.el:3:1:Warning: global/dynamic var `ll' lacks a prefix
eme.el:13:1:Error: Symbol's value as variable is void: ll
%

So the code has a problem, since byte-compiling it doesn't work
(emacs24 is 24.1, here). No wonder eager macro-expansion also leads
to problems.


Stefan

[Achim Gratz]

Stefan Monnier writes:
>> The following test case demonstrates a problem that has been distilled
>> from Org's test suite.
>
> % emacs24 -Q --batch -f batch-byte-compile eme.el
>
> In toplevel form:
> eme.el:3:1:Warning: global/dynamic var `ll' lacks a prefix
> eme.el:13:1:Error: Symbol's value as variable is void: ll
> %
>
> So the code has a problem, since byte-compiling it doesn't work

The code isn't meant to be byte-compiled, but if you want to I'd have to
split it into two seperate files. The ERT portion of the code is never
byte-compiled in Org and I don't know if it would make sense to do this.

> (emacs24 is 24.1, here). No wonder eager macro-expansion also leads
> to problems.

Well, the code does declare the variable symbol special and initializes
it nil, so finding the symbol undefined during compilation and/or macro
expansion would constitute a bug in either ERT or Emacs, no?


Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

[Stefan Monnier]

> Well, the code does declare the variable symbol special and initializes
> it nil, so finding the symbol undefined during compilation and/or macro
> expansion would constitute a bug in either ERT or Emacs, no?

The defvar is only executed at run time (although it does have an
effect at compile time, which is to tell the compiler that the variable
will exist at run time).

So using `ll' during the macro expansion is wrong.

If you want `ll' to defined earlier, you can wrap it in
`eval-and-compile' (tho it's better not to abuse it). I can't tell what
solution I'd recommend in your case, since your distilled test case is
"too distilled" to understand what it's trying to do.


Stefan

[Achim Gratz]

Stefan Monnier writes:
> The defvar is only executed at run time (although it does have an
> effect at compile time, which is to tell the compiler that the variable
> will exist at run time).
>
> So using `ll' during the macro expansion is wrong.

That may well be a bug in the original code, although of course the
defvar is in a different file that has been loaded before the test
definition would expand the macro, so the expectation is that the symbol
should exist and have nil value when trying to run the tests.

> If you want `ll' to defined earlier, you can wrap it in
> `eval-and-compile' (tho it's better not to abuse it). I can't tell what
> solution I'd recommend in your case, since your distilled test case is
> "too distilled" to understand what it's trying to do.

I'll have to check again how things were supposed to have been
initialized in Org, but the assumption that the (no longer existing)
macro definitions made was clearly that the stand-in for the ll symbol
was pre-existing at macro expansion time. I'll try to re-create the
test case to match that behaviour and come back to you.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

[Achim Gratz]

Stefan Monnier writes:
> So the code has a problem, since byte-compiling it doesn't work

> (emacs24 is 24.1, here). No wonder eager macro-expansion also leads
> to problems.

Here's the revised test case that compiles cleanly and still has the
same problem:

[Stefan Monnier]

> Here's the revised test case that compiles cleanly and still has the
> same problem:

> (defvar eme-ll nil)

> (defmacro one (p)
> `(progn (push ',p eme-ll)))

> (defmacro two (p)
> (let (pp)
> (setq pp (append eme-ll p))
> `(progn (push ',pp eme-ll))))

> (provide 'eme)


> (require 'eme)
> (require 'ert)


> (ert-deftest surprise ()
> (should
> (equal '((one . two) one)
> (progn
> (one one)
> (two two)
> eme-ll))))

I see the test fails, but that's just because the test is wrong.
Try to create a new file foo.el:

(require 'eme)

(message "Result = %s"
(progn
(one one)
(two two)
eme-ll))

Then byte-compile it. Then do

emacs23 --batch -Q -l ~/tmp/foo.el
and
emacs23 --batch -Q -l ~/tmp/foo.elc

You'll see that your code behaves differently when byte-compiled.


Stefan


Analysis:

(one one)

will add `one' to eme-ll at run-time.

(two two)

reads the macroexpansion-time (e.g. compilation-time, load-time, or
run-time) value of eme-ll and adds it to eme-ll at run-time.

eme-ll

returns the run-time value of eme-ll.

[Achim Gratz]

Stefan Monnier writes:
> I see the test fails, but that's just because the test is wrong.

[…]

> You'll see that your code behaves differently when byte-compiled.

Yes, we've already established that the original code itself had a bug.
The correct code would look like this

[Stefan Monnier]

retitle 14422 Apply eager-macroexpansion everywhere (eval-region, ...)
thanks

> The remaining point is that the ERT test still fails in exactly the same
> way when it is _not_ byte-compiled and batch-tested (like the original
> case in Org), but it produces the correct result when testing in
> interactive mode or in the debugger. I guess I'm asking for a warning
> for recursive macro expansions that manipulate the same variable both at
> expansion and at runtime in separate macros. Alternatively if the buggy
> code would always fail in the same way that would at least ensure it can
> be found more easily.

Indeed, that's part of the reason why I introduced this "eager
macroexpansion", which makes it that my previous foo.el test now behaves
identically whether it's byte-compiled or not.

The behavior is still different in a few other cases (where eager macro
expansion is not performed, typically M-C-x and things like that), but
I hope to reduce/eliminate them at some point.


Stefan

[Stefan Monnier]

> The behavior is still different in a few other cases (where eager macro
> expansion is not performed, typically M-C-x and things like that), but
> I hope to reduce/eliminate them at some point.

It just occurred to me that the patch below which I just installed
covers several of those cases.


Stefan


--- lisp/emacs-lisp/lisp-mode.el 2013-05-29 14:14:16 +0000
+++ lisp/emacs-lisp/lisp-mode.el 2013-06-03 14:23:31 +0000
@@ -809,6 +809,7 @@
(defun eval-sexp-add-defvars (exp &optional pos)
"Prepend EXP with all the `defvar's that precede it in the buffer.
POS specifies the starting position where EXP was found and defaults to point."
+ (setq exp (macroexpand-all exp)) ;Eager macro-expansion.
(if (not lexical-binding)
exp
(save-excursion

[Lars Ingebrigtsen]

It's unclear whether this fixes the reported bug? I tried the final
test case, and I didn't get any errors (but I'm not sure I'm testing
that the right way).

--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no