Fix use of macroexpand-all in loopy-iter and loopy.

CHANGELOG.md

@@ -48,6 +48,22 @@ This document describes the user-facing changes to Loopy.
 
 - Better signal an error with conflicting arguments in `numbers`.  See [#172].
 
+- Fix macro expansion in some cases by not resetting the macro environment
+  ([#173]).  For example, we failed to pass the current/modified environment to
+  `macroexpand-all` in some cases.
+
+  ```emacs-lisp
+  ;; Previously failed to use `cl-flet''s internally defined function
+  ;; for `10+':
+  ;; => (11 12 13 14 15)
+  (loopy (named outer)
+         (list i '((1 2) (3 4) (5)))
+         (loopy-iter (listing j i)
+                     (cl-flet ((10+ (y) (+ 10 y)))
+                       (at outer
+                           (collecting (10+ j))))))
+  ```
+
 ### Breaking Changes
 
 - Make it an error to re-use iteration variables with multiple iteration
@@ -172,6 +188,7 @@ This document describes the user-facing changes to Loopy.
 [#164]: https://github.com/okamsn/loopy/pull/164
 [#165]: https://github.com/okamsn/loopy/pull/165
 [#171]: https://github.com/okamsn/loopy/pull/172
+[#173]: https://github.com/okamsn/loopy/pull/173
 
 ## 0.11.2
 

README.org

@@ -56,6 +56,8 @@ please let me know.
    - The non-keyword arguments of =numbers= are deprecated.  Use the keyword
      arguments instead.  A =:test= keyword argument was added, which is more
      flexible and explicit than the non-keyword arguments.
+   - Fixed a problem with macro expansion in some cases for sub-macros
+     that created a new macro environment (e.g., ~cl-flet~).
  - Versions 0.11.1 and 0.11.2: None. Bug fixes.
  - Version 0.11.0:
    - More incorrect destructured bindings now correctly signal an error.

loopy-commands.el

@@ -114,6 +114,14 @@ The lists will be in the order parsed (correct for insertion)."
     ;; Return the sub-lists.
     (list (nreverse wrapped-main-body) (nreverse other-instructions))))
 
+;; We find ourselves doing this pattern a lot.
+(cl-defmacro loopy--bind-main-body ((main-expr other-instrs) value &rest body)
+  "Bind MAIN-EXPR and OTHER-INSTRS for those items in VALUE for BODY."
+  (declare (indent 2))
+  `(cl-destructuring-bind (,main-expr ,other-instrs)
+       (loopy--extract-main-body ,value)
+     ,@body))
+
 (defun loopy--convert-iteration-vars-to-other-vars (instructions)
   "Convert instructions for `loopy--iteration-vars' to `loopy--other-vars'.
 

loopy-misc.el

@@ -309,6 +309,7 @@ splitting (1 2 3) or (1 2 . 3) returns ((1 2) 3)."
 
 
 ;;;; Destructuring
+
 ;; This better allows for things to change in the future.
 (defun loopy--var-ignored-p (var)
   "Return whether VAR should be ignored."

loopy.el

@@ -654,6 +654,12 @@ code and must instead be cleaned up manually."
   (cl-callf2 seq-drop-while (lambda (x) (eq loopy--loop-name (caar x)))
              loopy--accumulation-variable-info))
 
+(defmacro loopy--with-protected-stack (&rest body)
+  "Protect the stack variables from BODY during unwind and cleanup."
+  `(unwind-protect
+       ,(macroexp-progn body)
+     (loopy--clean-up-stack-vars)))
+
 ;;;;; Process Instructions
 (cl-defun loopy--process-instruction (instruction &key erroring-instructions)
   "Process INSTRUCTION, assigning values to the variables in `loopy--variables'.
@@ -1003,7 +1009,8 @@ see the Info node `(loopy)' distributed with this package."
                 with macro-funcs = `(,@(cl-loop for i in loopy--suppressed-macros
                                                 collect (cons i nil))
                                      (loopy--optimized-accum
-                                      . loopy--expand-optimized-accum))
+                                      . loopy--expand-optimized-accum)
+                                     ,@macroexpand-all-environment)
                 for i in loopy--main-body
                 collect (macroexpand-all i macro-funcs)))
 

tests/iter-tests.el

@@ -762,8 +762,6 @@ E.g., \"(let ((for list)) ...)\" should not try to operate on the
                                  (push j target)))
                    target))))
 
-
-
 (ert-deftest loopy-iter-clean-stack-variables ()
   (let ((loopy--known-loop-names)
         (loopy--accumulation-places)

tests/tests.el

@@ -5549,6 +5549,96 @@ This assumes that you're on guix."
   :iter-bare ((list . listing)
               (cycle . cycling)))
 
+(loopy-deftest accum-flet-outside
+  :doc "Make sure that macro expansion doesn't mess with `cl-flet' environment.
+We don't want to rebind the environment to nil by failing to pass
+the existing environment (`macroexpand-all-environment') to
+`macroexpand-all'."
+  :result '(11 12 13 14 15)
+  :wrap ((x . `(cl-flet ((10+ (y) (+ 10 y))) ,x)))
+  :body ((list i '(1 2 3 4 5))
+         (collect (10+ i)))
+  :loopy t
+  :iter-keyword (list collect)
+  :iter-bare ((list . listing)
+              (collect . collecting)))
+
+(loopy-deftest accum-flet-outside-wrap-sma
+  :doc "Make sure that macro expansion doesn't mess with `cl-flet' environment.
+We don't want to rebind the environment to nil by failing to pass
+the existing environment (`macroexpand-all-environment') to
+`macroexpand-all'."
+  :result '(11 12 13 14 15)
+  :body ((wrap (cl-flet ((10+ (y) (+ 10 y)))))
+         (list i '(1 2 3 4 5))
+         (collect (10+ i)))
+  :loopy t
+  :iter-keyword (list collect)
+  :iter-bare ((list . listing)
+              (collect . collecting)))
+
+(loopy-deftest flet-iter-subloop
+  :doc "Make sure that macro expansion doesn't mess with `cl-flet' environment.
+We don't want to rebind the environment to nil by failing to pass
+the existing environment (`macroexpand-all-environment') to
+`macroexpand-all'."
+  :result '(11 12 13 14 15)
+  :multi-body t
+  :body [((named outer)
+          (list i '((1 2) (3 4) (5)))
+          (loopy-test-escape
+           (loopy-iter (listing j i)
+                       (at outer
+                           (cl-flet ((10+ (y) (+ 10 y)))
+                             (collecting (10+ j)))))))
+
+         ((named outer)
+          (list i '((1 2) (3 4) (5)))
+          (loopy-test-escape
+           (loopy-iter (listing j i)
+                       (at outer
+                           (cl-flet ((10+ (y) (+ 10 y)))
+                             (collecting (funcall #'10+ j)))))))
+
+         ((named outer)
+          (list i '((1 2) (3 4) (5)))
+          (loopy-test-escape
+           (loopy-iter (listing j i)
+                       (cl-flet ((10+ (y) (+ 10 y)))
+                         (at outer
+                             (collecting (10+ j)))))))
+
+         ((named outer)
+          (list i '((1 2) (3 4) (5)))
+          (loopy-test-escape
+           (loopy-iter (listing j i)
+                       (cl-flet ((10+ (y) (+ 10 y)))
+                         (at outer
+                             (collecting (10+ j)))))))
+
+         ((named outer)
+          (list i '((1 2) (3 4) (5)))
+          (loopy-test-escape
+           (loopy-iter (listing j i)
+                       (at outer
+                           (cl-flet ((10+ (y) (+ 10 y)))
+                             (collecting (funcall #'10+ j)))))))]
+  :loopy t
+  :iter-keyword (list collect)
+  :iter-bare ((list . listing)
+              (collect . collecting)))
+
+(loopy-deftest iter-list-in-top-level-expr
+  :doc "Macros that are required to be at the top level should not consider
+a sub-expression as the top level."
+  :error loopy-iteration-in-sub-level
+  :macroexpand t
+  :body ((let ((var 1))
+           (listing i '(1 2 3 4 5)))
+         (collecting i))
+  :iter-keyword (listing collecting)
+  :iter-bare t)
+
 ;; Local Variables:
 ;; End:
 ;; LocalWords:  destructurings backquote