procyberian/emacs
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

cl-symbol-macrolet: Fix recent regression

Browse source 
The recent fix for bug#57397 introduced a regression, breaking
the `cl-lib-symbol-macrolet-hide` test.  It turned out that the
origin of the problem was that `gv.el` uses `macroexpand-1` which
does not (can't) use `macroexpand` but `cl-symbol-macrolet` failed
to advise `macroexpand-1` the way it advised `macroexpand`.

To fix this, we change `cl-symbol-macrolet` so it advises both, and we
do that with a new `macroexpand` advice which delegates the bulk of
the work to `macroexpand-1`.

Along the way, I bumped into another bug in the interaction between
`cl-letf` and `cl-symbol-macrolet`, which I tried to fix in `cl-letf`.

I hear the war on `cl-symbol-macrolet` was a failure.
Maybe ... just say no?

* lisp/emacs-lisp/cl-macs.el (cl--sm-macroexpand-1): New function,
extracted from `cl--sm-macroexpand`.
(cl--sm-macroexpand): Rewrite completely.
(cl-symbol-macrolet): Advise both `macroexpand` and `macroexpand-1`.
(cl--letf): Don't use the "simple variable" code for symbol macros.

* test/lisp/emacs-lisp/cl-lib-tests.el (cl-lib-symbol-macrolet-hide):
Revert last change because the test was right.

* test/lisp/emacs-lisp/cl-macs-tests.el
(cl-macs-test--symbol-macrolet): Add a test case.
This commit is contained in:
Stefan Monnier 2022-09-06 00:08:35 -04:00
parent 9219e83b3c
commit 2a78f06ef4
3 changed files with 141 additions and 137 deletions
Download patch file Download diff file Expand all files Collapse all files

3
test/lisp/emacs-lisp/cl-lib-tests.el
View file

@ -511,9 +511,6 @@
(ert-deftest cl-lib-symbol-macrolet-hide ()
:expected-result :failed
;; FIXME -- it's unclear what the semantics here should be, but
;; 2dd1c2ab19f7fb99ecee flipped them.
;; bug#26325, bug#26073
(should (equal (let ((y 5))
(cl-symbol-macrolet ((x y))

9
test/lisp/emacs-lisp/cl-macs-tests.el
View file

@ -552,7 +552,14 @@ collection clause."
x)
x))
(error err))
'(1 7 3))))
'(1 7 3)))
(should (equal
(let ((x (list 42)))
(cl-symbol-macrolet ((m (car x)))
(list m
(cl-letf ((m 5)) m)
m)))
'(42 5 42))))
(ert-deftest cl-macs-loop-conditional-step-clauses ()
"These tests failed under the initial fixes in #bug#29799."