iterate: fix scope bug and declarations

Previously initforms were in the scope of the local function: now they
are not.

Declaration handling is better.

Author: tfeb

VERSION

@@ -1 +1 @@
-8.2.0
+8.2.1

iterate.lisp

@@ -3,9 +3,9 @@
 ;; Description       - Applicative iteration
 ;; Author            - Tim Bradshaw (tfb at lostwithiel)
 ;; Created On        - Sat Oct  7 00:23:24 2000
-;; Last Modified On  - Sat Mar 11 16:51:22 2023
+;; Last Modified On  - Wed Mar 20 08:41:14 2024
 ;; Last Modified By  - Tim Bradshaw (tfb at pendeen.fritz.box)
-;; Update Count      - 18
+;; Update Count      - 19
 ;; Status            - Unknown
 ;;
 ;; $Id$
@@ -14,14 +14,14 @@
 ;;;; * Applicative iteration (don't need this in CMUCL)
 ;;;
 
-;;; iterate.lisp is copyright 1997-2000, 2021, 2023 by me, Tim
+;;; iterate.lisp is copyright 1997-2000, 2021, 2023, 2024 by me, Tim
 ;;; Bradshaw, and may be used for any purpose whatsoever by anyone. It
 ;;; has no warranty whatsoever. I would appreciate acknowledgement if
 ;;; you use it in anger, and I would also very much appreciate any
 ;;; feedback or bug fixes.
 ;;;
-;;; The improvements to all this code in 2023, as well as the new
-;;; ITERATE*, ITERATING & ITERATING* are due to Zyni: thank you.
+;;; The improvements to all this code in 2023 & 2024, as well as the
+;;; new ITERATE*, ITERATING & ITERATING* are due to Zyni: thank you.
 ;;;
 
 (defpackage :org.tfeb.hax.iterate
@@ -33,19 +33,21 @@
 (provide :org.tfeb.hax.iterate)
 
 (defun extract-ignore/other-decls (decls/forms)
-  ;; See utilities.  Bit this is not the same: it returns all ignores and others as two values
-  ;; others as two values.
-  (do* ((ignores '())
-        (others '())
-        (tail decls/forms (rest tail))
-        (this (first tail) (first tail)))
-       ((or (null tail) (not (consp this))
-            (not (eql (first this) 'declare)))
-        (values (nreverse ignores) (nreverse others)))
-    (if (and (consp (second this))
-             (eql (first (second this)) 'ignore))
-        (push this ignores)
-      (push this others))))
+  ;; See utilities.  But this is not the same: it returns all ignores
+  ;; and others as two values What's returned is the bodies of two
+  ;; DECLARE forms.
+  (let ((ignores '())
+        (others '()))
+    (dolist (d/f decls/forms)
+      (unless (and (consp d/f)
+                   (eql (car d/f) 'declare))
+        (return))
+      (dolist (d (rest d/f))
+        (if (and (consp d) (eql (car d) 'ignore))
+            (push d ignores)
+          (push d others))))
+    (values (nreverse ignores)
+            (nreverse others))))
 
 (defun expand-iterate (name bindings body starred)
   (unless (every (lambda (binding)
@@ -67,30 +69,14 @@
                       binding)
                      (list
                       (first binding))))
-                 bindings))
-        (argvals
-         (mapcar (lambda (binding)
-                   (typecase binding
-                     (symbol
-                      nil)
-                     (list
-                      (case (length binding)
-                        ((1)
-                         nil)
-                        ((2)
-                         (second binding))))))
                  bindings)))
-    (if (not starred)
-        `(labels ((,name ,argnames
-                    ,@body))
-           (,name ,@argvals))
-      (multiple-value-bind (ignores others) (extract-ignore/other-decls body)
-        (declare (ignore others))
-        `(labels ((,name ,argnames
+    (multiple-value-bind (ignores others) (extract-ignore/other-decls body)
+      (declare (ignore ignores))
+      `(,(if starred 'let* 'let) ,bindings
+         (declare ,@others)
+         (labels ((,name ,argnames
                     ,@body))
-           (let* ,(mapcar #'list argnames argvals)
-             ,@ignores
-             (,name ,@argnames)))))))
+           (,name ,@argnames))))))
 
 (defmacro iterate (name bindings &body body)
   "Scheme-style named-LET: parallel binding
@@ -181,13 +167,13 @@ This is like LET*: initial values can depend on preceeding variables."
       (let ((secret-name (make-symbol (symbol-name name))))
         (multiple-value-bind (ignores others) (extract-ignore/other-decls body)
           `(labels ((,secret-name ,argnames
-                      ,@ignores ,@others
+                      (declare ,@ignores ,@others)
                       (flet ((,name (&key ,@(mapcar #'list argnames argsteps))
                                (,secret-name ,@argnames)))
                         (declare (inline ,name))
                         ,@body)))
              (let* ,(mapcar #'list argnames argvals)
-               ,@others
+               (declare ,@others)
                (,secret-name ,@argnames))))))))
 
 (defmacro iterating (name bindings &body body)
@@ -203,7 +189,7 @@ preceeding variables and step forms see the old values of variables."
   (expand-iterating name bindings body nil))
 
 (defmacro iterating* (name bindings &body body)
-  "Applicative iteration macro with optional step forms: sequentisl binding
+  "Applicative iteration macro with optional step forms: sequential binding
 
 This is like ITERATE but each binding can be (var init/step) or (var
 init step).  The local function has approproate keyword arguments

test/test-iterate.lisp

@@ -41,6 +41,30 @@
                 (declare (ignorable x))
                 y))))
 
+;;; Test that the scope of local functions is correct: they were wrong for a
+;;; very long time
+;;;
+
+(define-test ("org.tfeb.hax.iterate" "iterate local function not in scope of initforms")
+  (is eql 1
+      (let ((v 0))
+        (flet ((foo (x)
+                 (setf v x)))
+          (iterate foo ((x (foo 1)))
+            (declare (ignore x))
+            v)))))
+
+(define-test ("org.tfeb.hax.iterate" "iterate* local function not in scope of initforms")
+  (is-values
+      (let ((v 0))
+        (flet ((foo (x)
+                 (setf v x)))
+          (iterate* foo ((x (foo 1)) (y (1+ x)))
+            (declare (ignore x))
+            (values v y))))
+    (= 1)
+    (= 2)))
+
 (define-test ("org.tfeb.hax.iterate" "iterating basic")
   (is = 11 (iterating n ((i 0 (1+ i)))
              (if (> i 10) i (n))))