From d309cd32458183437ce40f6e5e64c2c01778e4e7 Mon Sep 17 00:00:00 2001 From: okamsn Date: Fri, 8 Mar 2024 22:23:38 -0500 Subject: [PATCH] Remove the deprecated `:result-type` keyword argument. This argument was deprecated in PR #162. See also issue #154. - Remove tests of `:result-type`. - Remove the relevant code. - Update test `acccumulation-conflicting-final-updates` to use custom commands instead of checking using `:result-type`. - Make sure the accumulation category actually gets set when the `:category` argument is passed to `loopy--defaccumulation`. --- loopy-commands.el | 114 ++++++++--------------------- tests/tests.el | 181 +++++++++++++++++----------------------------- 2 files changed, 96 insertions(+), 199 deletions(-) diff --git a/loopy-commands.el b/loopy-commands.el index 3989012a..6d1ed9ed 100644 --- a/loopy-commands.el +++ b/loopy-commands.el @@ -1378,9 +1378,8 @@ VARIABLE is the accumulation variable. CATEGORY is one of `list', `reverse-list', `string', `reverse-string', `vector', `reverse-vector', `boolean-thereis', `boolean-always-never', `number', and `generic'. It describes how the accumulation is -being built and its return type, ignoring special circumstances -like the `:result-type' keyword argument of commands like -`collect'. COMMAND is the accumulation command. +being built and its return type. COMMAND is the accumulation +command. - Strings are only made by `concat'. - Vectors are only made by `vconcat'. @@ -1698,7 +1697,9 @@ you can use in the instructions: (setq explicit-category category implicit-category category) (setq explicit-category (plist-get category :explicit) - implicit-category (plist-get category :implicit)))) + implicit-category (plist-get category :implicit))) + (unless (and explicit-category implicit-category) + (error "Explicit or implicit accumulation category is nil."))) `(let ((args) (opts)) ;; Compare with `loopy' for the equivalent code: @@ -1723,15 +1724,6 @@ you can use in the instructions: (ignore args opts) - (when (plist-member opts :result-type) - (warn "Loopy: `%s': Use of `:result-type' is deprecated. -Instead, use a coercing function like `seq-into' in a special -macro argument, such as `finally-return'. See also `accum-opt' at the Info node -`(loopy)Optimizing Accumulations'. -Warning trigger: %s" - name - cmd)) - (let ((arg-length (length args))) (cond ((= arg-length ,implicit-num-args) @@ -1820,8 +1812,7 @@ Warning trigger: %s" (defun loopy--construct-accum-adjoin (plist) "Construct optimized accumulation for `adjoin' from PLIST." (loopy--plist-bind ( :cmd cmd :loop loop :var var :val val - :test test :key key :at pos - :result-type (result-type 'list)) + :test test :key key :at pos) plist (map-let (('start start) ('end end)) @@ -1844,11 +1835,7 @@ Warning trigger: %s" (loopy--check-accumulation-compatibility loop var 'list cmd) `(,@(if (eq pos 'start) at-start-instrs - at-end-instrs) - (loopy--vars-final-updates - (,var . ,(if (eq 'list result-type) - nil - `(setq ,var (cl-coerce ,var (quote ,result-type)))))))) + at-end-instrs))) ;; Create list in reverse order. (loopy--check-accumulation-compatibility loop var 'reverse-list cmd) @@ -1856,23 +1843,16 @@ Warning trigger: %s" at-end-instrs at-start-instrs) (loopy--vars-final-updates - (,var . (setq ,var ,(if (eq 'list result-type) - `(nreverse ,var) - `(cl-coerce (nreverse ,var) - (quote ,result-type))))))))))))) + (,var . (setq ,var (nreverse ,var))))))))))) (loopy--defaccumulation adjoin - "Parse the `adjoin' command as (adjoin VAR VAL &key test key result-type at) - -RESULT-TYPE can be used to `cl-coerce' the return value." - :keywords (test key result-type at) + "Parse the `adjoin' command as (adjoin VAR VAL &key test key at)." + :keywords (test key at) ;; This is same as implicit behavior, so we only need to specify the explicit. :explicit - (loopy--plist-bind ( :test (test (quote #'equal)) :key key :at (pos 'end) - :result-type (result-type 'list)) + (loopy--plist-bind ( :test (test (quote #'equal)) :key key :at (pos 'end)) opts - (setq pos (loopy--normalize-symbol pos) - result-type (loopy--normalize-symbol result-type)) + (setq pos (loopy--normalize-symbol pos)) (when (eq pos 'beginning) (setq pos 'start)) (unless (memq pos '(start beginning end)) (signal 'loopy-bad-position-command-argument (list pos cmd))) @@ -1883,8 +1863,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `((loopy--main-body (loopy--optimized-accum '( :cmd ,cmd :name ,name :var ,var :val ,val - :test ,test :key ,key :at ,pos - :result-type ,result-type))))) + :test ,test :key ,key :at ,pos))))) (loopy--check-accumulation-compatibility loopy--loop-name var 'list cmd) `((loopy--accumulation-vars (,var nil)) @@ -1902,18 +1881,11 @@ RESULT-TYPE can be used to `cl-coerce' the return value." (loopy--produce-adjoin-end-tracking var val :test test :key key)) (t (signal 'loopy-bad-position-command-argument (list pos cmd)))) - (loopy--vars-final-updates - (,var . ,(if (eq result-type 'list) - nil - `(setq ,var (cl-coerce ,var - (quote ,(loopy--get-quoted-symbol - result-type)))))))))) + (loopy--vars-final-updates (,var . nil))))) :implicit - (loopy--plist-bind ( :test (test (quote #'equal)) :key key :at (pos 'end) - :result-type (result-type 'list)) + (loopy--plist-bind ( :test (test (quote #'equal)) :key key :at (pos 'end)) opts - (setq pos (loopy--normalize-symbol pos) - result-type (loopy--normalize-symbol result-type)) + (setq pos (loopy--normalize-symbol pos)) (when (eq pos 'beginning) (setq pos 'start)) (unless (memq pos '(start beginning end)) (signal 'loopy-bad-position-command-argument (list pos cmd))) @@ -1921,8 +1893,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `((loopy--main-body (loopy--optimized-accum '( :cmd ,cmd :name ,name :var ,var :val ,val - :test ,test :key ,key :at ,pos - :result-type ,result-type))) + :test ,test :key ,key :at ,pos))) (loopy--implicit-return ,var)))) ;;;;;;; Append @@ -2004,9 +1975,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." ;;;;;;; Collect (defun loopy--construct-accum-collect (plist) "Construct an optimized `collect' accumulation from PLIST." - (loopy--plist-bind ( :cmd cmd :loop loop :var var :val val - :at (pos 'end) - :result-type (result-type 'list)) + (loopy--plist-bind ( :cmd cmd :loop loop :var var :val val :at (pos 'end)) plist (setq pos (loopy--get-quoted-symbol pos)) `((loopy--accumulation-vars (,var nil)) @@ -2020,13 +1989,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `(,@(if (eq pos 'start) `((loopy--main-body (setq ,var (cons ,val ,var)))) (loopy--produce-collect-end-tracking var val)) - (loopy--vars-final-updates - (,var . ,(if (eq result-type 'list) - nil - `(setq ,var - (cl-coerce ,var - (quote ,(loopy--get-quoted-symbol - result-type))))))))) + (loopy--vars-final-updates (,var . nil)))) ;; Create list in reverse order. (loopy--check-accumulation-compatibility loop var 'reverse-list cmd) @@ -2034,21 +1997,14 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `((loopy--main-body (setq ,var (cons ,val ,var)))) (loopy--produce-collect-end-tracking var val)) (loopy--vars-final-updates - (,var . (setq ,var - ,(if (eq result-type 'list) - `(nreverse ,var) - `(cl-coerce - (nreverse ,var) - (quote ,(loopy--get-quoted-symbol result-type))))))))))))) + (,var . (setq ,var (nreverse ,var)))))))))) (loopy--defaccumulation collect - "Parse the `collect' command as (collect VAR VAL &key result-type at)." - :keywords (result-type at) - :explicit (loopy--plist-bind ( :at (pos (quote 'end)) - :result-type (result-type 'list)) + "Parse the `collect' command as (collect VAR VAL &key at)." + :keywords (at) + :explicit (loopy--plist-bind ( :at (pos (quote 'end))) opts - (setq pos (loopy--normalize-symbol pos) - result-type (loopy--normalize-symbol result-type)) + (setq pos (loopy--normalize-symbol pos)) (when (eq pos 'beginning) (setq pos 'start)) (unless (memq pos '(start beginning end)) (signal 'loopy-bad-position-command-argument (list pos cmd))) @@ -2058,8 +2014,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `((loopy--main-body (loopy--optimized-accum '( :loop ,loopy--loop-name :var ,var :val ,val - :cmd ,cmd :name ,name :at ,pos - :result-type ,result-type))))) + :cmd ,cmd :name ,name :at ,pos))))) (loopy--check-accumulation-compatibility loopy--loop-name var 'list cmd) `((loopy--accumulation-vars (,var nil)) @@ -2070,19 +2025,11 @@ RESULT-TYPE can be used to `cl-coerce' the return value." (loopy--produce-collect-end-tracking var val)) (t (signal 'loopy-bad-position-command-argument (list pos cmd)))) - (loopy--vars-final-updates - (,var . ,(if (eq result-type 'list) - nil - `(setq ,var - (cl-coerce ,var (quote - ,(loopy--get-quoted-symbol - result-type)))))))))) - - :implicit (loopy--plist-bind ( :at (pos 'end) - :result-type (result-type 'list)) + (loopy--vars-final-updates (,var . ,nil))))) + + :implicit (loopy--plist-bind ( :at (pos 'end)) opts - (setq pos (loopy--normalize-symbol pos) - result-type (loopy--normalize-symbol result-type)) + (setq pos (loopy--normalize-symbol pos)) (when (eq pos 'beginning) (setq pos 'start)) (unless (memq pos '(start beginning end)) (signal 'loopy-bad-position-command-argument (list pos cmd))) @@ -2090,8 +2037,7 @@ RESULT-TYPE can be used to `cl-coerce' the return value." `((loopy--main-body (loopy--optimized-accum '( :loop ,loopy--loop-name :var ,var :val ,val - :cmd ,cmd :name ,name :at ,pos - :result-type ,result-type))) + :cmd ,cmd :name ,name :at ,pos))) (loopy--implicit-return ,var)))) ;;;;;;; Concat diff --git a/tests/tests.el b/tests/tests.el index 9a13e0cc..28ad0e11 100644 --- a/tests/tests.el +++ b/tests/tests.el @@ -2841,23 +2841,75 @@ Using numbers directly will use less variables and more efficient code." ;;;; Accumulation Commands ;;;;; Final updates - (loopy-deftest accumulation-conflicting-final-updates + :doc "Check that commands of the same category but different updates error. + +Previously, this was mostly concerned with using a different +`:result-type' but in the same command type category. + +Wrapping with another eval to make sure variables are set by +expansion time." :error loopy-incompatible-accumulation-final-updates + :wrap ( + (x . `(cl-labels ((my-loopy-sum-command1 ((&whole cmd _ + var-or-val + &optional + maybe-val)) + "Set TARGET to the sum of ITEMS." + (let ((var) + (val)) + (if maybe-val + (setq var var-or-val + val maybe-val) + (setq var 'loopy-result + val var-or-val)) + (loopy--check-accumulation-compatibility + loopy--loop-name + var 'number cmd) + `((loopy--accumulation-vars (,var nil)) + (loopy--main-body (setq ,var (+ ,var ,val))) + (loopy--vars-final-updates + (,var . (setq ,var (1- ,var))))))) + (my-loopy-sum-command2 ((&whole cmd _ + var-or-val + &optional + maybe-val)) + "Set TARGET to the sum of ITEMS." + (let ((var) + (val)) + (if maybe-val + (setq var var-or-val + val maybe-val) + (setq var 'loopy-result + val var-or-val)) + (loopy--check-accumulation-compatibility + loopy--loop-name + var 'number cmd) + `((loopy--accumulation-vars (,var nil)) + (loopy--main-body (setq ,var (+ ,var ,val))) + (loopy--vars-final-updates + (,var . (setq ,var (- ,var 100)))))))) + (let ((loopy-command-parsers + (thread-first loopy-command-parsers + (map-insert 'sum1 + #'my-loopy-sum-command1) + (map-insert 'sum2 + #'my-loopy-sum-command2))) + (loopy-iter-bare-commands (append '(sum1 sum2) + loopy-iter-bare-commands))) + (eval (quote ,x) t))))) :multi-body t - :body [((list i '((1) (2) (3))) - (collect i) - (collect (1+ i) :result-type vector)) + :body [((list i '(1 2 3 4 5)) + (sum1 my-target i) + (sum2 my-target i) + (finally-return my-target)) - ((list i '((1) (2) (3))) - (collect coll2 i) - (collect (1+ i) :result-type vector :into coll2))] + ((list i '(1 2 3 4 5)) + (sum1 i) + (sum2 i))] :loopy t - :iter-keyword (list append collect vconcat) - :iter-bare ((list . listing) - (append . appending) - (collect . collecting) - (vconcat . vconcating))) + :iter-keyword (sum1 sum2) + :iter-bare t) ;;;;; Into Argument (loopy-deftest accumulation-into-argument @@ -2887,6 +2939,7 @@ Using numbers directly will use less variables and more efficient code." ;;;;; Command Compatibility (loopy-deftest accumulation-compatibility-lists + :doc "Check that commands with different accumulation types should raise error." :should t :macroexpand t :body ((list i '((1 2 3) (4 5 6))) @@ -2908,6 +2961,7 @@ Using numbers directly will use less variables and more efficient code." (nconc . nconcing))) (loopy-deftest accumulation-compatibility-different-types + :doc "Check that commands with different accumulation types should raise error." :error loopy-incompatible-accumulations :macroexpand t :multi-body t @@ -3136,46 +3190,6 @@ Using numbers directly will use less variables and more efficient code." :iter-bare ((list . listing) (adjoin . adjoining))) -(loopy-deftest adjoin-coerce-array/vector - :result [1 2 3 4 5] - :multi-body t - :body [((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type 'array)) - ((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type array)) - ((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type 'vector)) - ((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type vector))] - :loopy t - :iter-keyword (list adjoin) - :iter-bare ((list . listing) - (adjoin . adjoining))) - -(loopy-deftest adjoin-coerce-list - :result '(1 2 3 4 5) - :multi-body t - :body [((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type 'list)) - ((list i '(1 2 2 3 4 4 5)) - (adjoin i :result-type list))] - :loopy t - :iter-keyword (list adjoin) - :iter-bare ((list . listing) - (adjoin . adjoining))) - -(loopy-deftest adjoin-coerce-string - :result "abcd" - :multi-body t - :body [((list i '(?a ?b ?c ?d)) - (adjoin i :result-type 'string)) - ((list i '(?a ?b ?c ?d)) - (adjoin i :result-type string))] - :loopy t - :iter-keyword (list adjoin) - :iter-bare ((list . listing) - (adjoin . adjoining))) - (loopy-deftest adjoin-:at-end :result '(1 2 3 4 5) :multi-body t @@ -3495,69 +3509,6 @@ Using `start' and `end' in either order should give the same result." :iter-bare ((list . listing) (collect . collecting))) -(loopy-deftest collect-coercion-vector - :result [1 2 3] - :multi-body t - :body [((list j '(1 2 3)) - (collect v j :result-type 'vector) - (finally-return v)) - - ((list j '(1 2 3)) - (collect v j :result-type vector) - (finally-return v)) - - ((list j '(1 2 3)) - (collect j :result-type 'vector)) - - ((list j '(1 2 3)) - (collect j :result-type vector))] - :loopy t - :iter-keyword (list collect) - :iter-bare ((list . listing) - (collect . collecting))) - -(loopy-deftest collect-coercion-list - :result '(1 2 3) - :multi-body t - :body [((list j '(1 2 3)) - (collect v j :result-type 'list) - (finally-return v)) - - ((list j '(1 2 3)) - (collect v j :result-type list) - (finally-return v)) - - ((list j '(1 2 3)) - (collect j :result-type 'list)) - - ((list j '(1 2 3)) - (collect j :result-type list))] - :loopy t - :iter-keyword (list collect) - :iter-bare ((list . listing) - (collect . collecting))) - -(loopy-deftest collect-coercion-string - :result "abc" - :multi-body t - :body [((list j '(?a ?b ?c)) - (collect v j :result-type 'string) - (finally-return v)) - - ((list j '(?a ?b ?c)) - (collect v j :result-type string) - (finally-return v)) - - ((list j '(?a ?b ?c)) - (collect j :result-type 'string)) - - ((list j '(?a ?b ?c)) - (collect j :result-type string))] - :loopy t - :iter-keyword (list collect) - :iter-bare ((list . listing) - (collect . collecting))) - (loopy-deftest collect-:at-start/beginning-explicit :result (list (list 3 2 1) (list 1 2 3)) :multi-body t