Fix some bugs with find and nil values. (#171)

- 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.

CHANGELOG.md

@@ -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

loopy-commands.el

@@ -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

tests/tests.el

@@ -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)
@@ -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)