Skip to content

Commit

Permalink
Fix some bugs with find.
Browse files Browse the repository at this point in the history
- Fix `:on-failure` when the accumulated value is set to nil.  Previously,
  a nil value was interpreted a failing the test, which meant that the value
  `:on-failure` would be used.

- Fix detecting `:on-failure` when the keyword value is nil.  Previously, if the
  value given by `:on-failure` was nil, then that would be interpreted as the
  keyword not being used.
  • Loading branch information
okamsn committed Sep 17, 2023
1 parent a337730 commit e4892f5
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 35 deletions.
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,31 @@ This document describes the user-facing changes to Loopy.
(reduce i #'*))
```

- Fix `find` when `:on-failure` is nil. Previously, `nil` was interpreted as
not passing `:on-failure`.

```emacs-lisp
;; Previously erroneously returned 27:
;; => nil
(loopy (with (val 27))
(list i '(1 2 3))
(find val nil (> i 10) :on-failure nil)
(finally-return val))
```

- Fix `find` when `EXPR` is nil and `:on-failure` is given. Previously, after
the test passed and `VAR` was set to `nil`, that `nil` was interpreted as not
passing the test, so that `VAR` then bound to the value passed for
`:on-failure`.

```emacs-lisp
;; Previously erroneously returned 27:
;; => nil
(loopy (list i '(1 2 3))
(find nil (> i 1) :on-failure 27))
```


### Breaking Changes

- Make it an error to re-use iteration variables with multiple iteration
Expand Down
44 changes: 29 additions & 15 deletions loopy-commands.el
Original file line number Diff line number Diff line change
Expand Up @@ -2244,35 +2244,49 @@ This function is called by `loopy--expand-optimized-accum'."
(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 (plist-get opts :on-failure))
(tag-name (loopy--produce-non-returning-exit-tag-name
loopy--loop-name)))
loopy--loop-name))
(found (gensym "found")))

`((loopy--non-returning-exit-used ,tag-name)
(loopy--accumulation-vars (,var nil))
(loopy--main-body (when ,test-form
(setq ,var ,val)
(throw (quote ,tag-name) t)))
;; If VAR nil, bind to ON-FAILURE.
,(when on-failure
`(loopy--vars-final-updates
(,var . (if ,var nil (setq ,var ,on-failure)))))))
,@(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)))))))
: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 (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))
(loopy--main-body (when ,test-form
(setq ,var ,val)
(throw (quote ,tag-name) t)))
;; If VAR nil, bind to ON-FAILURE.
,(when on-failure
`(loopy--vars-final-updates
(,var . (if ,var nil (setq ,var ,on-failure)))))
,@(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))))

;;;;;;; Set Accum
Expand Down
108 changes: 88 additions & 20 deletions tests/tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -4991,8 +4991,13 @@ Not multiple of 3: 7"
;;;;; finding
(loopy-deftest find-pass-notest
:result 3
:body ((list i '(1 2 3))
(_cmd i (> i 2)))
:multi-body t
:body [((list i '(1 2 3))
(_cmd res i (> i 2))
(finally-return res))

((list i '(1 2 3))
(_cmd i (> i 2)))]
:repeat _cmd
:loopy ((_cmd . (find finding)))
:iter-keyword ((list . list)
Expand All @@ -5002,68 +5007,131 @@ Not multiple of 3: 7"

(loopy-deftest find-fail-notest
:result nil
:body ((list i '(1 2 3))
(find i (> i 4)))
:multi-body t
:body [((list i '(1 2 3))
(find res i (> i 4))
(finally-return res))

((list i '(1 2 3))
(find i (> i 4)))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-fail-onfail
:result 0
:body ((list i '(1 2 3))
(find i (> i 4) :on-failure 0))
:multi-body t
:body [((list i '(1 2 3))
(find res i (> i 4) :on-failure 0)
(finally-return res))

((list i '(1 2 3))
(find i (> i 4) :on-failure 0))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-pass-onfail
:result 2
:body ((list i '(1 2 3))
(find i (> i 1) :on-failure 0))
:multi-body t
:body [((list i '(1 2 3))
(find res i (> i 1) :on-failure 0)
(finally-return res))

((list i '(1 2 3))
(find i (> i 1) :on-failure 0))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-expr-onfail-explicit-nil
:doc "Make sure a nil value will still be set on failure."
:result nil
:multi-body t
:body [((with (val 27))
(list i '(1 2 3))
(find val nil (> i 10) :on-failure nil)
(finally-return val))

((with (loopy-result 27))
(list i '(1 2 3))
(find nil (> i 10) :on-failure nil))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-pass-var
:result 2
:body ((list i '(1 2 3))
(find found i (= i 2))
(finally-return found))
:multi-body t
:body [((list i '(1 2 3))
(find found i (= i 2))
(finally-return found))

((list i '(1 2 3))
(find i (= i 2)))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-fail-var
:result nil
:body ((list i '(1 2 3))
(find found i (> i 3))
(finally-return found))
:multi-body t
:body [((list i '(1 2 3))
(find found i (> i 3))
(finally-return found))

((list i '(1 2 3))
(find i (> i 3)))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-fail-var-onfail
:result "not found"
:body ((list i '(1 2 3))
(find whether-found i (> i 4)
:on-failure "not found")
(finally-return whether-found))
:multi-body t
:body [((list i '(1 2 3))
(find whether-found i (> i 4) :on-failure "not found")
(finally-return whether-found))

((list i '(1 2 3))
(find i (> i 4) :on-failure "not found"))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-pass-var-onfail
:result 2
:body ((list i '(1 2 3))
(find whether-found i (> i 1)
:multi-body t
:body [((list i '(1 2 3))
(find whether-found i (> i 1)
:on-failure "not found")
(finally-return whether-found))

((list i '(1 2 3))
(find i (> i 1) :on-failure "not found"))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
(find . finding)))

(loopy-deftest find-expr-is-nil-onfail
:doc "Make sure a nil value not assumed to be a failure."
:result nil
:multi-body t
:body [((list i '(1 2 3))
(find whether-found nil (> i 1)
:on-failure "not found")
(finally-return whether-found))

((list i '(1 2 3))
(find nil (> i 1) :on-failure "not found"))]
:loopy t
:iter-keyword (list find)
:iter-bare ((list . listing)
Expand Down

0 comments on commit e4892f5

Please sign in to comment.