MKCL icon indicating copy to clipboard operation
MKCL copied to clipboard

`walker:macroexpand-all` and `tagbody`

Open Gleefre opened this issue 1 year ago • 3 comments

There are two problems caused by not treating forms in the tagbody correctly.

  1. walker:macroexpand-all expands symbols that have a symbol-macro definition in tagbody when they are in the tag position.
  2. walker:macroexpand-all doesn't "protect" macroexpansion of the macros in tagbody that can become a tag (symbols and integers).
(require :walker)
(walker:macroexpand-all
 '(symbol-macrolet ((tag (tag-expanded)))
   (tagbody tag)))
; => (SYMBOL-MACROLET ((TAG (TAG-EXPANDED)))
;      (TAGBODY (TAG-EXPANDED)))

(walker:macroexpand-all
 '(macrolet ((not-tag () 'tag-expanded))
   (tagbody (not-tag))))
; => (MACROLET ((NOT-TAG ()
;                 'TAG-EXPANDED))
;      (TAGBODY (NOT-TAG)))

The determination of which elements of the body are tags and which are statements is made prior to any macro expansion of that element. If a statement is a macro form and its macro expansion is an atom, that atom is treated as a statement, not a tag.

-- CLHS, tagbody: https://www.lispworks.com/documentation/HyperSpec/Body/s_tagbod.htm

That means that

  1. tag in tagbody should not be expanded.
  2. (not-tag) expansion should be "protected" so that it doesn't become a tag, for example via progn.

Tested on MKCL 1.1.11

(lisp-implementation-version)  ; => "1.1.11.188-f731a69"

This affects quite a few other implementations, see this table (two last columns): https://plaster.tymoon.eu/view/4637.

Gleefre avatar Nov 01 '24 01:11 Gleefre

You are using a version of MKCL master/head that is a bit more that a year old. Why don't you pull the current MKCL master/head such that we have a common base to discuss from? I'll be investigating all this in the next few days...

Thank you for the issue report.

jcbeaudoin avatar Nov 01 '24 03:11 jcbeaudoin

Sorry, forgot that I have built mkcl from master/head at some point. I have updated it to current head, same results with the macroexpand-all test.

(lisp-implementation-version)  ; => "1.1.11.206-b414811"

By the way, there are a few more issues with macroexpand-all, should I open separate issues or add them all here?

Gleefre avatar Nov 01 '24 13:11 Gleefre

Open separate issues if they are significant enough please.

jcbeaudoin avatar Nov 01 '24 17:11 jcbeaudoin