Skip to content

Commit

Permalink
Fix reduce with no explicit initialization. (#164)
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.  Instead, it stores the first value. 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 420ed83
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 420ed83

Please sign in to comment.