Skip to content

Commit

Permalink
Fix reduce with no explicit initialization.
Browse files Browse the repository at this point in the history
Change so that it does not pass the first value of `EXPR` to the function.  This
change makes it work more like `cl-reduce`, which is the expected behavior.
  • Loading branch information
okamsn committed Aug 27, 2023
1 parent 9cae74c commit ad091a3
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 27 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ This document describes the user-facing changes to Loopy.
`collect`. This change makes these commands work with the new system for
optimized accumulation variables.

- Without an explicit starting value for the accumulation variable, `reduce` now
uses the first accumulated value without passing it to the function. This is
how reducing actually works, and was the intended behavior. Previously, the
function was always called. _This is a breaking change._ See [#164].

```emacs-lisp
;; Now correct behavior (would previously error):
;; => 6
(loopy (list i '(1 2 3))
(reduce i #'*))
```

### Breaking Changes

- Make it an error to re-use iteration variables with multiple iteration
Expand Down Expand Up @@ -93,6 +105,9 @@ This document describes the user-facing changes to Loopy.
`loopy--iteration-vars`. For example, these are the variables bound by the
`set` command, which are allowed to occur in more than one command.

[#164]: https://github.com/okamsn/loopy/pull/164


## 0.11.2

### Bugs Fixed
Expand Down
3 changes: 3 additions & 0 deletions README.org
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ please let me know.
special macro arguments, possibly with =accum-opt=.
- The =:init= keyword argument is deprecated. Use the special macro argument
=with= instead.
- =reduce= has been fixed. It now works like ~cl-reduce~ when the variable
starting value isn't explicitly given, storing the first value instead of
storing the result of passing the first value and ~nil~ to the function.
- Versions 0.11.1 and 0.11.2: None. Bug fixes.
- Version 0.11.0:
- More incorrect destructured bindings now correctly signal an error.
Expand Down
36 changes: 22 additions & 14 deletions doc/loopy-doc.org
Original file line number Diff line number Diff line change
Expand Up @@ -2347,32 +2347,40 @@ using the =set= command.

#+findex: reduce
#+findex: reducing
- =(reduce VAR EXPR FUNC)= :: Reduce =EXPR= into =VAR= by =FUNC=. =FUNC= is
called with =VAR= as the first argument and =EXPR= as the second argument.
This is unlike =accumulate=, which gives =VAR= and =EXPR= to =FUNC= in the
opposite order (that is, =EXPR= first, then =VAR=).
- =(reduce VAR EXPR FUNC)= :: Reduce =EXPR= into =VAR= by =FUNC=, like in
~cl-reduce~. =FUNC= is called with =VAR= as the first argument and =EXPR= as
the second argument. This is unlike =accumulate=, which gives =VAR= and
=EXPR= to =FUNC= in the opposite order (that is, =EXPR= first, then =VAR=).

This command also has the alias =reducing=.

When =VAR= does not have an explicit starting value (given with the special
macro argument =with=), the first accumulated value is =EXPR=. The first
accumulated value is not the result of passing =VAR= and =EXPR= to =FUNC=.
Using the =with= special macro argument is similar to using ~cl-reduce~'s
=:initial-value= keyword argument.

This command is similar in effect to the =set= command.

#+begin_src emacs-lisp
;; => 6
(loopy (list i '(1 2 3))
(reduce i #'*))

;; Similar to the above:
(loopy (list i '(1 2 3))
(set loopy-result i (* i loopy-result))
(finally-return loopy-result))

;; = > 6
(loopy (with (my-reduction 0))
(list i '(1 2 3))
(reduce my-reduction i #'+)
(finally-return my-reduction))

;; Works similarly to above:
(loopy (with (my-reduction 0))
(list i '(1 2 3))
(set my-reduction (+ i my-reduction))
(finally-return my-reduction))

;; => 24
(loopy (with (loopy-result 1))
(list i '(1 2 3 4))
(reduce i #'*))
;; Similar to the above:
(cl-reduce #'+ (list 1 2 3) :initial-value 0)
(seq-reduce #'+ [1 2 3] 0)
#+end_src

This command also has the alias =callf=. It is similar to using the
Expand Down
59 changes: 49 additions & 10 deletions loopy-commands.el
Original file line number Diff line number Diff line change
Expand Up @@ -1773,6 +1773,12 @@ accumulation variable. The default accumulation variable is
'loopy-result))
(val (cl-first args)))
(ignore var val)

(when (and (loopy--with-bound-p var)
(plist-member opts :init))
(error "Loopy: Can't use `:init' and `with' for same variable: %s"
var))

;; Substitute in the instructions.
;;
;; If `:into' is used, then we must act as if this is the
Expand Down Expand Up @@ -1806,6 +1812,8 @@ accumulation variable. The default accumulation variable is
(let ((var (cl-first args))
(val (cl-second args)))
(ignore var val)


(if (sequencep var)
;; If we need to destructure the sequence `var', we use the
;; function named by
Expand All @@ -1814,6 +1822,12 @@ accumulation variable. The default accumulation variable is
(funcall (or loopy--destructuring-accumulation-parser
#'loopy--parse-destructuring-accumulation-command)
cmd)

(when (and (loopy--with-bound-p var)
(plist-member opts :init))
(error "Loopy: Can't use `:init' and `with' for same variable: %s"
var))

;; Substitute in the instructions.
,(when category
`(loopy--check-accumulation-compatibility
Expand Down Expand Up @@ -2515,19 +2529,44 @@ This function is used by `loopy--expand-optimized-accum'."
With INIT, initialize VAR to INIT. Otherwise, VAR starts as nil."
:num-args 3
:keywords (init)
:category generic
:implicit (loopy--plist-bind (:init init) opts
(loopy--check-accumulation-compatibility
loopy--loop-name var 'generic cmd)
`((loopy--accumulation-vars (,var ,init))
(loopy--main-body
(setq ,var ,(loopy--apply-function (cl-second args) var val)))
`(,@(cond
((loopy--with-bound-p var)
`((loopy--main-body
(setq ,var ,(loopy--apply-function (cl-second args) var val)))))
((plist-member opts :init)
`((loopy--accumulation-vars (,var ,init))
(loopy--main-body
(setq ,var ,(loopy--apply-function (cl-second args) var val)))))
(t
(let ((first-time (gensym "first-time")))
`((loopy--accumulation-vars (,var ,init))
(loopy--accumulation-vars (,first-time t))
(loopy--main-body
(if ,first-time
(setq ,first-time nil
,var ,val)
(setq ,var ,(loopy--apply-function (cl-second args) var val))))))))
(loopy--implicit-return ,var)))
:explicit (loopy--plist-bind (:init init) opts
(loopy--check-accumulation-compatibility
loopy--loop-name var 'generic cmd)
`((loopy--accumulation-vars (,var ,init))
(loopy--main-body
(setq ,var ,(loopy--apply-function (cl-third args) var val))))))
`(,@(cond
((loopy--with-bound-p var)
`((loopy--main-body
(setq ,var ,(loopy--apply-function (cl-third args) var val)))))
((plist-member opts :init)
`((loopy--accumulation-vars (,var ,init))
(loopy--main-body
(setq ,var ,(loopy--apply-function (cl-third args) var val)))))
(t
(let ((first-time (gensym "first-time")))
`((loopy--accumulation-vars (,var ,init))
(loopy--accumulation-vars (,first-time t))
(loopy--main-body
(if ,first-time
(setq ,first-time nil
,var ,val)
(setq ,var ,(loopy--apply-function (cl-third args) var val)))))))))))

;;;;;;; Sum
(loopy--defaccumulation sum
Expand Down
35 changes: 32 additions & 3 deletions tests/tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -4160,6 +4160,24 @@ Using `start' and `end' in either order should give the same result."
:iter-bare ((list . listing)
(_cmd . (reducing))))

(loopy-deftest reduce-no-init
:doc "When the accumulation variable isn't explicitly initialized,
`reduce' should store the first value without calling the function.

This is how `cl-reduce' and `seq-reduce' work."
:result (cl-reduce #'+ '(1 2 3))
:multi-body t
:body [((list i '(1 2 3))
(reduce r i #'+)
(finally-return r))

((list i '(1 2 3))
(reduce i #'+))]
:loopy t
:iter-keyword (list reduce)
:iter-bare ((list . listing)
(reduce . reducing)))

(loopy-deftest reduce-with
:doc "Test that we can replace `:init' with `with'."
:result 6
Expand Down Expand Up @@ -4198,9 +4216,20 @@ Using `start' and `end' in either order should give the same result."

(loopy-deftest reduce-destructuring-+
:result '(4 6)
:body ((list i '((1 2) (3 4)))
(reduce (r1 r2) i #'+ :init 0)
(finally-return r1 r2))
:multi-body t
:body [((with (r1 0)
(r2 0))
(list i '((1 2) (3 4)))
(reduce (r1 r2) i #'+)
(finally-return r1 r2))

((list i '((1 2) (3 4)))
(reduce (r1 r2) i #'+ :init 0)
(finally-return r1 r2))

((list i '((1 2) (3 4)))
(reduce (r1 r2) i #'+)
(finally-return r1 r2))]
:loopy t
:iter-keyword (list reduce)
:iter-bare ((list . listing)
Expand Down

0 comments on commit ad091a3

Please sign in to comment.