From 1fdb5d8cfe85829b1ecc798790e59e0612bd8627 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 14 Sep 2024 14:45:11 +0000 Subject: [PATCH] (GitHub Action) Review evaluation of keyword arguments during expansion and run time. (#211) This commit was copied from the master branch. Commit: 7d76cee0a1c05f64457466fc183bc51497bc2633 Author: okamsn <28612288+okamsn@users.noreply.github.com> Date: 2024-09-14 14:45:00 +0000 Review evaluation of keyword arguments during expansion and run time. (#211) - Make `close` in `iter` evaluable. Make it evaluated at the beginning. - Make `on-failure` in `find` evaluated at the beginning. Closes issue #210. See this PR #211. --- CHANGELOG.md | 14 ++++++-- README.org | 11 ++++-- doc/loopy-doc.org | 43 ++++++++++++++++++---- doc/loopy.texi | 51 ++++++++++++++++++++------ loopy-commands.el | 91 +++++++++++++++++++++++++++-------------------- tests/tests.el | 48 +++++++++++++++++++++++++ 6 files changed, 198 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4041636e..21101b55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,8 +42,15 @@ This document describes the user-facing changes to Loopy. - Make `sequence-index` the default name and `seq-index` an alias ([#126, #206]). -- Allow the `unique` keyword argument of the commands `map` and `map-ref` to be - evaluable at run time, instead of just checked at compile time ([#209]). +- Review when the values of keyword arguments are taken and used ([#210]): + - Make the `close` keyword argument of the `iter` command able to be evaluated + during run time ([#211]). It was previously only used during macro expansion. + - Make the `close` keyword argument of the `iter` command evaluated at the + start of the loop to be consistent with other commands ([#211]). + - Make the `on-failure` keyword argument of the `find` command evaluated at the + start of the loop to be consistent with other commands ([#211]). + - Allow the `unique` keyword argument of the commands `map` and `map-ref` to be + evaluable at run time, instead of just checked at compile time ([#209]). ### Improvements @@ -60,7 +67,8 @@ This document describes the user-facing changes to Loopy. [#206]: https://github.com/okamsn/loopy/pull/206 [#207]: https://github.com/okamsn/loopy/pull/207 [#209]: https://github.com/okamsn/loopy/pull/209 - +[#210]: https://github.com/okamsn/loopy/issues/210 +[#211]: https://github.com/okamsn/loopy/pull/211 ## 0.13.0 diff --git a/README.org b/README.org index a7c39ea1..b21b9b2d 100644 --- a/README.org +++ b/README.org @@ -39,8 +39,15 @@ please let me know. list of built-in aliases in the future. They can still be added to the list of known aliases using ~loopy-defalias~. See the changelog for more information. - - The =:unique= keyword argument of the =map= and =map-ref= commands can now - be evaluable at run time, similar to most other keyword arguments. + - Improved consistency of some keyword arguments: + - The =:unique= keyword argument of the =map= and =map-ref= commands can now + be evaluable at run time. + - The =:close= argument of the =iter= command is now evaluable, instead of + only being used during macro expansion. + - The =:close= argument of the =iter= command is now evaluated at the + beginning of the loop. + - The =:on-failure= argument of the =find= command is now evaluated at the + beginning of the loop. - Version 0.13.0: - The deprecated =:init= keyword argument has been removed. Use the =with= special macro argument instead. diff --git a/doc/loopy-doc.org b/doc/loopy-doc.org index d1c59495..a97ee9e9 100644 --- a/doc/loopy-doc.org +++ b/doc/loopy-doc.org @@ -1507,13 +1507,17 @@ loop. This restriction allows for producing more efficient code. holds the value yielded by the iterator. The loop ends when the iterator finishes. + =close= is whether the generator should be closed via ~iter-close~ after the + loop ends. The default is ~t~. Note that Emacs will eventually close + un-closed, un-reachable generators during garbage collection. To be + consistent with other commands, =close= is evaluated at the start of the loop, + even though it's value is only used after the loop finishes. + =yield-result= is the optional second argument to the function ~iter-next~, which is the value of ~iter-yield~ in the iterator (not to be confused with - the value yielded by calling ~iter-next~). - - =close= is whether the generator should be closed via ~iter-close~ after the - loop ends. The default is ~t~. Note that Emacs will eventually close - un-closed, un-reachable generators during garbage collection. + the value yielded by calling ~iter-next~). Unlike =close=, which is evaluated + once, =yield-result= is an expression which is substituted into the loop body. + Therefore, =yield-result= can be used to repeatedly call functions. For efficiency, when possible, =VAR= is bound to the yielded value before each step of the loop, which is used to detect whether the iterator signals that it @@ -1526,6 +1530,7 @@ loop. This restriction allows for producing more efficient code. #+begin_src emacs-lisp ;; With var: + ;; ;; => ((1 . 4) (2 . 5) (3 . 6)) (loopy (with (iter-maker (iter-lambda (x) (while x @@ -1535,6 +1540,7 @@ loop. This restriction allows for producing more efficient code. (collect (cons i j))) ;; Without var: + ;; ;; => (1 2 3) (loopy (iter (funcall (iter-lambda () ;; These yielded values are all ignored. @@ -1543,6 +1549,21 @@ loop. This restriction allows for producing more efficient code. (iter-yield 'third-yield)))) (set i 1 (1+ i)) (collect i)) + + ;; Using `yield-result': + ;; + ;; => (3 2 1) + (loopy (with (yield-results nil)) + (set i 1 (1+ i)) + (iter (funcall (iter-lambda () + ;; The value from the expression specified by + ;; `:yield-result' is `push'-ed: + (push (iter-yield 'first-yield) yield-results) + (push (iter-yield 'second-yield) yield-results) + (push (iter-yield 'third-yield) yield-results))) + ;; Note that the value of `i' evaluated each time: + :yield-result i) + (finally-return yield-results)) #+end_src #+ATTR_TEXINFO: :tag Warning @@ -2124,7 +2145,7 @@ source sequences. =EXPR=. If =by= is non-nil (default: 1), then move to the next n-th element during each iteration. This command is a special case of the =substream= command (described below), setting =VAR= to the first element of each - substream. For more information, see the command =substream=. + substream. For more information on streams, see the command =substream=. This command also has the alias =streaming=. @@ -3397,6 +3418,9 @@ Sequence accumulation commands are used to join lists (such as =union= and normal failure and =VAR= will be set to the value of =ON-FAILURE=, if provided. + To be consistent with other commands, =ON-FAILURE= is evaluated at the + start of the loop, even though that is not necessarily where it is used. + #+BEGIN_SRC emacs-lisp ;; => (13 (1 2)) (loopy (list i '(1 2 3 4 5 6 7 8)) @@ -3420,6 +3444,13 @@ Sequence accumulation commands are used to join lists (such as =union= and ;; => nil (loopy (list i '(1 2 3 4 5 6)) (find nil (> i 3) :on-failure 27)) + + ;; Value of `:on-failure' gotten at the start of the loop: + ;; => 27 + (loopy (with (on-fail 27)) + (list i '(1 2 3)) + (set on-fail 33) + (find i (> i 4) :on-failure on-fail)) #+END_SRC *** Optimizing Accumulations diff --git a/doc/loopy.texi b/doc/loopy.texi index 5d04971d..dce352f0 100644 --- a/doc/loopy.texi +++ b/doc/loopy.texi @@ -705,7 +705,7 @@ You should keep in mind that commands are evaluated in order. This means that attempting something like the below example might not do what you expect, as @samp{i} is assigned a value from the list after collecting @samp{i} into @samp{coll}. -@float Listing,orgfa9540f +@float Listing,orgcd9d132 @lisp ;; => (nil 1 2) (loopy (collect coll i) @@ -887,7 +887,7 @@ the flag @samp{dash} provided by the package @samp{loopy-dash}. Below are two examples of destructuring in @code{cl-loop} and @code{loopy}. -@float Listing,orgb706e77 +@float Listing,orgf148300 @lisp ;; => (1 2 3 4) (cl-loop for (i . j) in '((1 . 2) (3 . 4)) @@ -902,7 +902,7 @@ Below are two examples of destructuring in @code{cl-loop} and @code{loopy}. @caption{Destructuring values in a list.} @end float -@float Listing,orgcc60e47 +@float Listing,orgff97ac8 @lisp ;; => (1 2 3 4) (cl-loop for elem in '((1 . 2) (3 . 4)) @@ -1668,13 +1668,17 @@ iterator object produced by a calling a generator function. If given, @samp{VAR holds the value yielded by the iterator. The loop ends when the iterator finishes. +@samp{close} is whether the generator should be closed via @code{iter-close} after the +loop ends. The default is @code{t}. Note that Emacs will eventually close +un-closed, un-reachable generators during garbage collection. To be +consistent with other commands, @samp{close} is evaluated at the start of the loop, +even though it's value is only used after the loop finishes. + @samp{yield-result} is the optional second argument to the function @code{iter-next}, which is the value of @code{iter-yield} in the iterator (not to be confused with -the value yielded by calling @code{iter-next}). - -@samp{close} is whether the generator should be closed via @code{iter-close} after the -loop ends. The default is @code{t}. Note that Emacs will eventually close -un-closed, un-reachable generators during garbage collection. +the value yielded by calling @code{iter-next}). Unlike @samp{close}, which is evaluated +once, @samp{yield-result} is an expression which is substituted into the loop body. +Therefore, @samp{yield-result} can be used to repeatedly call functions. For efficiency, when possible, @samp{VAR} is bound to the yielded value before each step of the loop, which is used to detect whether the iterator signals that it @@ -1687,6 +1691,7 @@ This command also has the name @samp{iterating}. @lisp ;; With var: +;; ;; => ((1 . 4) (2 . 5) (3 . 6)) (loopy (with (iter-maker (iter-lambda (x) (while x @@ -1696,6 +1701,7 @@ This command also has the name @samp{iterating}. (collect (cons i j))) ;; Without var: +;; ;; => (1 2 3) (loopy (iter (funcall (iter-lambda () ;; These yielded values are all ignored. @@ -1704,6 +1710,21 @@ This command also has the name @samp{iterating}. (iter-yield 'third-yield)))) (set i 1 (1+ i)) (collect i)) + +;; Using `yield-result': +;; +;; => (3 2 1) +(loopy (with (yield-results nil)) + (set i 1 (1+ i)) + (iter (funcall (iter-lambda () + ;; The value from the expression specified by + ;; `:yield-result' is `push'-ed: + (push (iter-yield 'first-yield) yield-results) + (push (iter-yield 'second-yield) yield-results) + (push (iter-yield 'third-yield) yield-results))) + ;; Note that the value of `i' evaluated each time: + :yield-result i) + (finally-return yield-results)) @end lisp @quotation Warning @@ -2306,7 +2327,7 @@ Iterate through the elements for the stream @samp{EXPR}. If @samp{by} is non-nil (default: 1), then move to the next n-th element during each iteration. This command is a special case of the @samp{substream} command (described below), setting @samp{VAR} to the first element of each -substream. For more information, see the command @samp{substream}. +substream. For more information on streams, see the command @samp{substream}. This command also has the alias @samp{streaming}. @@ -3678,6 +3699,9 @@ If the loop is left early and @samp{TEST} was never non-nil, this is the same as normal failure and @samp{VAR} will be set to the value of @samp{ON-FAILURE}, if provided. +To be consistent with other commands, @samp{ON-FAILURE} is evaluated at the +start of the loop, even though that is not necessarily where it is used. + @lisp ;; => (13 (1 2)) (loopy (list i '(1 2 3 4 5 6 7 8)) @@ -3701,6 +3725,13 @@ provided. ;; => nil (loopy (list i '(1 2 3 4 5 6)) (find nil (> i 3) :on-failure 27)) + +;; Value of `:on-failure' gotten at the start of the loop: +;; => 27 +(loopy (with (on-fail 27)) + (list i '(1 2 3)) + (set on-fail 33) + (find i (> i 4) :on-failure on-fail)) @end lisp @end table @@ -4654,7 +4685,7 @@ using the @code{let*} special form. This method recognizes all commands and their aliases in the user option @code{loopy-aliases}. -@float Listing,orgab882ee +@float Listing,org7411b6a @lisp ;; => ((1 2 3) (-3 -2 -1) (0)) (loopy-iter (arg accum-opt positives negatives other) diff --git a/loopy-commands.el b/loopy-commands.el index 0778c4ef..e06c9b8e 100644 --- a/loopy-commands.el +++ b/loopy-commands.el @@ -782,8 +782,9 @@ is a function by which to update VAR (default `cdr')." VAR is the variable. VAL is an generator-producing function, as from `iter-defun' or `iter-lambda'. YIELD-RESULT is the optional -value of `iter-next'. CLOSE is whether the iterator should be -closed after the loop completes." +value of `iter-next' and is an expression, not a value. CLOSE is +whether the iterator should be closed after the loop completes +and is a value." :required-vals 0 ;; Require values /after/ `var'. :other-vals (0 1) :keywords (:yield-result :close) @@ -825,11 +826,16 @@ closed after the loop completes." (iter-next ,obj-holder ,yield-result) (iter-end-of-sequence nil) (:success t)))))) - ,(when close - `(loopy--vars-final-updates - (,obj-holder . (iter-close ,obj-holder)))))))) - - + ,@(pcase close + ('nil nil) + ('t `((loopy--vars-final-updates + (,obj-holder . (iter-close ,obj-holder))))) + (_ + (loopy--instr-let-const* ((close2 close)) + loopy--iteration-vars + `((loopy--vars-final-updates + (,obj-holder . (when ,close2 + (iter-close ,obj-holder)))))))))))) ;;;;;; List ;; TODO: Make this a normal function. @@ -2197,57 +2203,64 @@ This function is called by `loopy--expand-optimized-accum'." ;;;;;;; Find (loopy--defaccumulation find - "Parse a command of the form `(finding VAR EXPR TEST &key ON-FAILURE)'." + "Parse a command of the form `(finding VAR EXPR TEST &key ON-FAILURE)'. + +ON-FAILURE is evaluated at the beginning of the loop, even though +it is only meaningful at the end." :num-args 3 :keywords (on-failure) :explicit (let* ((test-arg (cl-third args)) (test-form (if (loopy--quoted-form-p test-arg) `(,(loopy--get-function-symbol test-arg) ,val) test-arg)) - (of-used (plist-member opts :on-failure)) + (on-failure-given (plist-member opts :on-failure)) (on-failure (plist-get opts :on-failure)) (tag-name (loopy--produce-non-returning-exit-tag-name loopy--loop-name)) (found (gensym "found"))) - `((loopy--non-returning-exit-used ,tag-name) - (loopy--accumulation-vars (,var nil)) - ,@(if of-used - ;; If TEST always nil, bind to ON-FAILURE. - `((loopy--accumulation-vars (,found nil)) - (loopy--main-body (when ,test-form - (setq ,var ,val - ,found t) - (throw (quote ,tag-name) t))) - (loopy--vars-final-updates - (,var . (unless ,found (setq ,var ,on-failure))))) - `((loopy--main-body (when ,test-form - (setq ,var ,val) - (throw (quote ,tag-name) t))))))) + (loopy--instr-let-const* ((on-failure2 on-failure)) + loopy--accumulation-vars + `((loopy--non-returning-exit-used ,tag-name) + (loopy--accumulation-vars (,var nil)) + ,@(if on-failure-given + ;; If TEST always nil, bind to ON-FAILURE. + `((loopy--accumulation-vars (,found nil)) + (loopy--main-body (when ,test-form + (setq ,var ,val + ,found t) + (throw (quote ,tag-name) t))) + (loopy--vars-final-updates + (,var . (unless ,found (setq ,var ,on-failure2))))) + `((loopy--main-body (when ,test-form + (setq ,var ,val) + (throw (quote ,tag-name) t)))))))) :implicit (let* ((test-arg (cl-second args)) (test-form (if (loopy--quoted-form-p test-arg) `(,(loopy--get-function-symbol test-arg) ,val) test-arg)) - (of-used (plist-member opts :on-failure)) + (on-failure-given (plist-member opts :on-failure)) (on-failure (plist-get opts :on-failure)) (found (gensym "found")) (tag-name (loopy--produce-non-returning-exit-tag-name loopy--loop-name))) - `((loopy--non-returning-exit-used ,tag-name) - (loopy--accumulation-vars (,var nil)) - ,@(if of-used - ;; If TEST always nil, bind to ON-FAILURE. - `((loopy--accumulation-vars (,found nil)) - (loopy--main-body (when ,test-form - (setq ,var ,val - ,found t) - (throw (quote ,tag-name) t))) - (loopy--vars-final-updates - (,var . (unless ,found (setq ,var ,on-failure))))) - `((loopy--main-body (when ,test-form - (setq ,var ,val) - (throw (quote ,tag-name) t))))) - (loopy--implicit-return ,var)))) + (loopy--instr-let-const* ((on-failure2 on-failure)) + loopy--accumulation-vars + `((loopy--non-returning-exit-used ,tag-name) + (loopy--accumulation-vars (,var nil)) + ,@(if on-failure-given + ;; If TEST always nil, bind to ON-FAILURE. + `((loopy--accumulation-vars (,found nil)) + (loopy--main-body (when ,test-form + (setq ,var ,val + ,found t) + (throw (quote ,tag-name) t))) + (loopy--vars-final-updates + (,var . (unless ,found (setq ,var ,on-failure2))))) + `((loopy--main-body (when ,test-form + (setq ,var ,val) + (throw (quote ,tag-name) t))))) + (loopy--implicit-return ,var))))) ;;;;;;; Set Accum (loopy--defaccumulation set-accum diff --git a/tests/tests.el b/tests/tests.el index bccb18c2..baa4976b 100644 --- a/tests/tests.el +++ b/tests/tests.el @@ -1489,6 +1489,34 @@ Using numbers directly will use less variables and more efficient code." (collect . collecting) (leave . leaving))) +(loopy-deftest iter-close-value + :doc "Check that `close' is respected when it's a variable." + :result t + :body (loopy (with (some-val nil) + (iter-maker (iter-lambda () + (iter-yield 1) + (iter-yield 2) + (iter-yield 3))) + (gen (funcall iter-maker))) + (iter i gen :close some-val) + (leave) ; Should not preventing closing or final updates. + (finally-return + ;; Older versions of Emacs don't have the `:success' clause, + ;; so we work around it. + (let ((val (condition-case err + (iter-next gen) + (iter-end-of-sequence 'fail) + (error + (signal (car err) (cdr err)))))) + (if (eq val 'fail) + nil + (iter-close gen) + t)))) + :loopy t + :iter-keyword (iter leave) + :iter-bare ((iter . iterating) + (leave . leaving))) + (loopy-deftest iter-same-gen :doc "Check that `iter' doesn't reset iterator objects." :result '((1 . 2) (3 . 4)) @@ -5677,6 +5705,26 @@ Not multiple of 3: 7" :iter-bare ((list . listing) (find . finding))) +(loopy-deftest find-onfail-at-beginning + :doc "Make sure `:on-failure' is evaluated at the beginning." + :result 27 + :multi-body t + :body [((with (on-fail 27)) + (list i '(1 2 3)) + (set on-fail 33) + (find res i (> i 4) :on-failure on-fail) + (finally-return res)) + + ((with (on-fail 27)) + (list i '(1 2 3)) + (set on-fail 33) + (find i (> i 4) :on-failure on-fail))] + :loopy t + :iter-keyword (list set find) + :iter-bare ((list . listing) + (set . setting) + (find . finding))) + (loopy-deftest find-fail-onfail :result 0 :multi-body t