From 6535fd1fa9ac21238a168916249ac59677a6118e Mon Sep 17 00:00:00 2001 From: akater Date: Tue, 20 Jul 2021 01:25:01 +0000 Subject: Evaluate eql specializers * lisp/emacs-lisp/cl-generic.el (cl-generic-generalizers): Evaluate forms that are eql specializers. Provide backward compatibility with a warning. * test/lisp/emacs-lisp/cl-generic-tests.el: Add a test. * lisp/emacs-lisp/bindat.el (bindat--type): Adhere to the new rule. * lisp/emacs-lisp/edebug.el (edebug--match-&-spec-op): Adhere to the new rule. * lisp/emacs-lisp/map.el (map-into): Adhere to the new rule. * lisp/emacs-lisp/radix-tree.el (map-into): Adhere to the new rule. * lisp/frame.el (cl-generic-define-context-rewriter): Adhere to the new rule. * lisp/gnus/gnus-search.el (gnus-search-transform-expression): Adhere to the new rule. * lisp/image/image-converter.el (image-converter--probe image-converter--convert): Adhere to the new rule. * lisp/mail/smtpmail.el (smtpmail-try-auth-method): Adhere to the new rule. * lisp/progmodes/elisp-mode.el (xref-backend-definitions) (xref-backend-apropos): Adhere to the new rule. * lisp/progmodes/etags.el (xref-backend-identifier-at-point) (xref-backend-identifier-completion-table) (xref-backend-identifier-completion-ignore-case) (xref-backend-definitions)(xref-backend-apropos): Adhere to the new rule. * test/lisp/emacs-lisp/checkdoc-tests.el (checkdoc-cl-defmethod-with-types-ok) (checkdoc-cl-defmethod-qualified-ok) (checkdoc-cl-defmethod-with-extra-qualifier-ok): Adhere to the new rule. * etc/NEWS: Describe the change. --- lisp/emacs-lisp/map.el | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lisp/emacs-lisp/map.el') diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el index 5c76fb9eb95..c59342875db 100644 --- a/lisp/emacs-lisp/map.el +++ b/lisp/emacs-lisp/map.el @@ -407,15 +407,15 @@ See `map-into' for all supported values of TYPE." "Convert MAP into a map of TYPE.") ;; FIXME: I wish there was a way to avoid this η-redex! -(cl-defmethod map-into (map (_type (eql list))) +(cl-defmethod map-into (map (_type (eql 'list))) "Convert MAP into an alist." (map-pairs map)) -(cl-defmethod map-into (map (_type (eql alist))) +(cl-defmethod map-into (map (_type (eql 'alist))) "Convert MAP into an alist." (map-pairs map)) -(cl-defmethod map-into (map (_type (eql plist))) +(cl-defmethod map-into (map (_type (eql 'plist))) "Convert MAP into a plist." (let (plist) (map-do (lambda (k v) (setq plist `(,v ,k ,@plist))) map) @@ -510,7 +510,7 @@ KEYWORD-ARGS are forwarded to `make-hash-table'." map) ht)) -(cl-defmethod map-into (map (_type (eql hash-table))) +(cl-defmethod map-into (map (_type (eql 'hash-table))) "Convert MAP into a hash-table with keys compared with `equal'." (map--into-hash map (list :size (map-length map) :test #'equal))) -- cgit v1.2.3 From 37d48edf6d406a4730caa0393f7695de2bfadfcc Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Wed, 4 Aug 2021 00:48:50 +0100 Subject: Fix merging of ambiguous nil maps * lisp/emacs-lisp/map.el: Bump version to 3.1. (map--merge): New merging subroutine that uses a hash table in place of lists, for both efficiency and avoiding ambiguities (bug#49848). (map-merge): Rewrite in terms of map--merge. (map-merge-with): Ditto. This ensures that FUNCTION is called whenever two keys are merged, even if they are not eql (which could happen until now). It also makes map-merge-with consistent with map-merge, thus achieving greater overall predictability. * etc/NEWS: Announce this weakening of guarantees. * test/lisp/emacs-lisp/map-tests.el (test-map-merge) (test-map-merge-with): Don't depend on specific orderings. Test that nil is correctly merged into a plist. --- etc/NEWS | 8 ++++++ lisp/emacs-lisp/map.el | 60 ++++++++++++++++++++++++--------------- test/lisp/emacs-lisp/map-tests.el | 24 ++++++++++------ 3 files changed, 61 insertions(+), 31 deletions(-) (limited to 'lisp/emacs-lisp/map.el') diff --git a/etc/NEWS b/etc/NEWS index 366ea1abd6d..a321ffd81f1 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1636,6 +1636,14 @@ This is a slightly deeper copy than the previous 'copy-sequence'. --- *** The function 'map-contains-key' now supports plists. +--- +*** More consistent duplicate key handling in 'map-merge-with'. +Until now, 'map-merge-with' promised to call its function argument +whenever multiple maps contained 'eql' keys. However, this did not +always coincide with the keys that were actually merged, which could +be 'equal' instead. The function argument is now called whenever keys +are merged, for greater consistency with 'map-merge' and 'map-elt'. + ** Package --- diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el index c59342875db..988a62a4e34 100644 --- a/lisp/emacs-lisp/map.el +++ b/lisp/emacs-lisp/map.el @@ -5,7 +5,7 @@ ;; Author: Nicolas Petton ;; Maintainer: emacs-devel@gnu.org ;; Keywords: extensions, lisp -;; Version: 3.0 +;; Version: 3.1 ;; Package-Requires: ((emacs "26")) ;; This file is part of GNU Emacs. @@ -371,37 +371,51 @@ The default implementation delegates to `map-do'." map) t)) +(defun map--merge (merge type &rest maps) + "Merge into a map of TYPE all the key/value pairs in MAPS. +MERGE is a function that takes the target MAP, a KEY, and a +VALUE, merges KEY and VALUE into MAP, and returns the result. +MAP may be of a type other than TYPE." + ;; Use a hash table internally if `type' is a list. This avoids + ;; both quadratic lookup behavior and the type ambiguity of nil. + (let* ((tolist (memq type '(list alist plist))) + (result (map-into (pop maps) + ;; Use same testfn as `map-elt' gv setter. + (cond ((eq type 'plist) '(hash-table :test eq)) + (tolist '(hash-table :test equal)) + (type))))) + (dolist (map maps) + (map-do (lambda (key value) + (setq result (funcall merge result key value))) + map)) + ;; Convert internal representation to desired type. + (if tolist (map-into result type) result))) + (defun map-merge (type &rest maps) "Merge into a map of TYPE all the key/value pairs in MAPS. See `map-into' for all supported values of TYPE." - (let ((result (map-into (pop maps) type))) - (while maps - ;; FIXME: When `type' is `list', we get an O(N^2) behavior. - ;; For small tables, this is fine, but for large tables, we - ;; should probably use a hash-table internally which we convert - ;; to an alist in the end. - (map-do (lambda (key value) - (setf (map-elt result key) value)) - (pop maps))) - result)) + (apply #'map--merge + (lambda (result key value) + (setf (map-elt result key) value) + result) + type maps)) (defun map-merge-with (type function &rest maps) "Merge into a map of TYPE all the key/value pairs in MAPS. -When two maps contain the same (`eql') key, call FUNCTION on the two +When two maps contain the same key, call FUNCTION on the two values and use the value returned by it. Each of MAPS can be an alist, plist, hash-table, or array. See `map-into' for all supported values of TYPE." - (let ((result (map-into (pop maps) type)) - (not-found (list nil))) - (while maps - (map-do (lambda (key value) - (cl-callf (lambda (old) - (if (eql old not-found) - value - (funcall function old value))) - (map-elt result key not-found))) - (pop maps))) - result)) + (let ((not-found (list nil))) + (apply #'map--merge + (lambda (result key value) + (cl-callf (lambda (old) + (if (eql old not-found) + value + (funcall function old value))) + (map-elt result key not-found)) + result) + type maps))) (cl-defgeneric map-into (map type) "Convert MAP into a map of TYPE.") diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el index a04c6bef02a..658ed2e7119 100644 --- a/test/lisp/emacs-lisp/map-tests.el +++ b/test/lisp/emacs-lisp/map-tests.el @@ -446,16 +446,24 @@ Evaluate BODY for each created map." (ert-deftest test-map-merge () "Test `map-merge'." - (should (equal (map-merge 'list '(a 1) '((b . 2) (c . 3)) - #s(hash-table data (c 4))) - '((c . 4) (b . 2) (a . 1))))) + (should (equal (sort (map-merge 'list '(a 1) '((b . 2) (c . 3)) + #s(hash-table data (c 4))) + (lambda (x y) (string< (car x) (car y)))) + '((a . 1) (b . 2) (c . 4)))) + (should (equal (map-merge 'list () '(:a 1)) '((:a . 1)))) + (should (equal (map-merge 'alist () '(:a 1)) '((:a . 1)))) + (should (equal (map-merge 'plist () '(:a 1)) '(:a 1)))) (ert-deftest test-map-merge-with () - (should (equal (map-merge-with 'list #'+ - '((1 . 2)) - '((1 . 3) (2 . 4)) - '((1 . 1) (2 . 5) (3 . 0))) - '((3 . 0) (2 . 9) (1 . 6))))) + (should (equal (sort (map-merge-with 'list #'+ + '((1 . 2)) + '((1 . 3) (2 . 4)) + '((1 . 1) (2 . 5) (3 . 0))) + #'car-less-than-car) + '((1 . 6) (2 . 9) (3 . 0)))) + (should (equal (map-merge-with 'list #'+ () '(:a 1)) '((:a . 1)))) + (should (equal (map-merge-with 'alist #'+ () '(:a 1)) '((:a . 1)))) + (should (equal (map-merge-with 'plist #'+ () '(:a 1)) '(:a 1)))) (ert-deftest test-map-merge-empty () "Test merging of empty maps." -- cgit v1.2.3 From fffcc7ab25021fd9d73d50cf685a77777d38265c Mon Sep 17 00:00:00 2001 From: Lars Ingebrigtsen Date: Wed, 1 Sep 2021 10:32:49 +0200 Subject: Fix (setf (map-elt map key) (my-func)) * lisp/emacs-lisp/map.el (map-elt): Ensure that the value isn't referenced more than once (bug#50290). --- lisp/emacs-lisp/map.el | 18 ++++++++++-------- test/lisp/emacs-lisp/map-tests.el | 9 +++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) (limited to 'lisp/emacs-lisp/map.el') diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el index 988a62a4e34..77431f0c594 100644 --- a/lisp/emacs-lisp/map.el +++ b/lisp/emacs-lisp/map.el @@ -119,14 +119,16 @@ or array." ((key key) (default default) (testfn testfn)) (funcall do `(map-elt ,mgetter ,key ,default) (lambda (v) - `(condition-case nil - ;; Silence warnings about the hidden 4th arg. - (with-no-warnings (map-put! ,mgetter ,key ,v ,testfn)) - (map-not-inplace - ,(funcall msetter - `(map-insert ,mgetter ,key ,v)) - ;; Always return the value. - ,v)))))))) + (macroexp-let2 nil v v + `(condition-case nil + ;; Silence warnings about the hidden 4th arg. + (with-no-warnings + (map-put! ,mgetter ,key ,v ,testfn)) + (map-not-inplace + ,(funcall msetter + `(map-insert ,mgetter ,key ,v)) + ;; Always return the value. + ,v))))))))) ;; `testfn' is deprecated. (advertised-calling-convention (map key &optional default) "27.1")) ;; Can't use `cl-defmethod' with `advertised-calling-convention'. diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el index 658ed2e7119..c0f0dbc92be 100644 --- a/test/lisp/emacs-lisp/map-tests.el +++ b/test/lisp/emacs-lisp/map-tests.el @@ -521,5 +521,14 @@ Evaluate BODY for each created map." 'value2)) (should (equal (map-elt ht 'key) 'value2)))) +(ert-deftest test-setf-map-with-function () + (let ((num 0) + (map nil)) + (setf (map-elt map 'foo) + (funcall (lambda () + (cl-incf num)))) + ;; Check that the function is only called once. + (should (= num 1)))) + (provide 'map-tests) ;;; map-tests.el ends here -- cgit v1.2.3 From 14495e33afa0b8038c494f1e6e90065683ccbd07 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Wed, 15 Sep 2021 00:17:26 +0100 Subject: Consistently test alist keys with equal in map.el * etc/NEWS: Announce new default behavior of map-elt and map-delete on alists. * lisp/emacs-lisp/map.el: Bump to version 3.2. (map-elt): Test alist keys with equal by default. Betray a little bit more information in the docstring on which functions are used for which map types. (Bug#47368) (map-put): Update docstring accordingly. (map--plist-delete): Consistently test plist keys with eq. (map-delete): Consistently test alist keys with equal. * test/lisp/emacs-lisp/map-tests.el (test-map-elt-testfn): Update for new map-elt behavior. (test-map-put!-alist, test-map-delete-alist): New tests. --- etc/NEWS | 6 ++++++ lisp/emacs-lisp/map.el | 18 +++++++++++------- test/lisp/emacs-lisp/map-tests.el | 34 +++++++++++++++++++++++++++++----- 3 files changed, 46 insertions(+), 12 deletions(-) (limited to 'lisp/emacs-lisp/map.el') diff --git a/etc/NEWS b/etc/NEWS index d80b9c0e66f..d241f864d35 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -3706,6 +3706,12 @@ and well-behaved enough to lose the "internal" marker. ** map.el +--- +*** Alist keys are now consistently compared with 'equal' by default. +Until now, 'map-elt' and 'map-delete' compared alist keys with 'eq' by +default. They now use 'equal' instead, for consistency with +'map-put!' and 'map-contains-key'. + *** Pcase 'map' pattern added keyword symbols abbreviation. A pattern like '(map :sym)' binds the map's value for ':sym' to 'sym', equivalent to '(map (:sym sym))'. diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el index 77431f0c594..e0af448eafc 100644 --- a/lisp/emacs-lisp/map.el +++ b/lisp/emacs-lisp/map.el @@ -5,7 +5,7 @@ ;; Author: Nicolas Petton ;; Maintainer: emacs-devel@gnu.org ;; Keywords: extensions, lisp -;; Version: 3.1 +;; Version: 3.2 ;; Package-Requires: ((emacs "26")) ;; This file is part of GNU Emacs. @@ -103,10 +103,14 @@ Returns the result of evaluating the form associated with MAP-VAR's type." (and (consp list) (atom (car list)))) (cl-defgeneric map-elt (map key &optional default testfn) - "Lookup KEY in MAP and return its associated value. + "Look up KEY in MAP and return its associated value. If KEY is not found, return DEFAULT which defaults to nil. -TESTFN is deprecated. Its default depends on the MAP argument. +TESTFN is the function to use for comparing keys. It is +deprecated because its default and valid values depend on the MAP +argument. Generally, alist keys are compared with `equal', plist +keys with `eq', and hash-table keys with the hash-table's test +function. In the base definition, MAP can be an alist, plist, hash-table, or array." @@ -136,7 +140,7 @@ or array." :list (if (map--plist-p map) (let ((res (plist-member map key))) (if res (cadr res) default)) - (alist-get key map default nil testfn)) + (alist-get key map default nil (or testfn #'equal))) :hash-table (gethash key map default) :array (if (map-contains-key map key) (aref map key) @@ -147,7 +151,7 @@ or array." If KEY is already present in MAP, replace the associated value with VALUE. When MAP is an alist, test equality with TESTFN if non-nil, -otherwise use `eql'. +otherwise use `equal'. MAP can be an alist, plist, hash-table, or array." (declare (obsolete "use map-put! or (setf (map-elt ...) ...) instead" "27.1")) @@ -157,7 +161,7 @@ MAP can be an alist, plist, hash-table, or array." (let ((tail map) last) (while (consp tail) (cond - ((not (equal key (car tail))) + ((not (eq key (car tail))) (setq last tail) (setq tail (cddr last))) (last @@ -177,7 +181,7 @@ Keys not present in MAP are ignored.") ;; FIXME: Signal map-not-inplace i.s.o returning a different list? (if (map--plist-p map) (map--plist-delete map key) - (setf (alist-get key map nil t) nil) + (setf (alist-get key map nil t #'equal) nil) map)) (cl-defmethod map-delete ((map hash-table) key) diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el index c0f0dbc92be..afade8e295b 100644 --- a/test/lisp/emacs-lisp/map-tests.el +++ b/test/lisp/emacs-lisp/map-tests.el @@ -85,11 +85,13 @@ Evaluate BODY for each created map." (should (= 5 (map-elt map 0 5))))) (ert-deftest test-map-elt-testfn () - (let ((map (list (cons "a" 1) (cons "b" 2))) - ;; Make sure to use a non-eq "a", even when compiled. - (noneq-key (string ?a))) - (should-not (map-elt map noneq-key)) - (should (map-elt map noneq-key nil #'equal)))) + (let* ((a (string ?a)) + (map `((,a . 0) (,(string ?b) . 1)))) + (should (= (map-elt map a) 0)) + (should (= (map-elt map "a") 0)) + (should (= (map-elt map (string ?a)) 0)) + (should (= (map-elt map "b") 1)) + (should (= (map-elt map (string ?b)) 1)))) (ert-deftest test-map-elt-with-nil-value () (should-not (map-elt '((a . 1) (b)) 'b 2))) @@ -129,6 +131,19 @@ Evaluate BODY for each created map." (setf (map-elt map size) 'v) (should (eq (map-elt map size) 'v)))))) +(ert-deftest test-map-put!-alist () + "Test `map-put!' test function on alists." + (let ((key (string ?a)) + (val 0) + map) + (should-error (map-put! map key val) :type 'map-not-inplace) + (setq map (list (cons key val))) + (map-put! map key (1- val)) + (should (equal map '(("a" . -1)))) + (map-put! map (string ?a) (1+ val)) + (should (equal map '(("a" . 1)))) + (should-error (map-put! map (string ?a) val #'eq) :type 'map-not-inplace))) + (ert-deftest test-map-put-alist-new-key () "Regression test for Bug#23105." (let ((alist (list (cons 0 'a)))) @@ -197,6 +212,15 @@ Evaluate BODY for each created map." (with-empty-maps-do map (should (eq map (map-delete map t))))) +(ert-deftest test-map-delete-alist () + "Test `map-delete' test function on alists." + (let* ((a (string ?a)) + (map `((,a) (,(string ?b))))) + (setq map (map-delete map a)) + (should (equal map '(("b")))) + (setq map (map-delete map (string ?b))) + (should-not map))) + (ert-deftest test-map-nested-elt () (let ((vec [a b [c d [e f]]])) (should (eq (map-nested-elt vec '(2 2 0)) 'e))) -- cgit v1.2.3 From fa92b040c6738de7278605cadeace0c5380a0814 Mon Sep 17 00:00:00 2001 From: Stefan Monnier Date: Tue, 28 Sep 2021 10:29:27 -0400 Subject: * lisp/emacs-lisp/map.el: Restore compatibility with Emacs-26 Don't use the new `eql` syntax. --- lisp/emacs-lisp/map.el | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'lisp/emacs-lisp/map.el') diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el index e0af448eafc..da4502f9ed8 100644 --- a/lisp/emacs-lisp/map.el +++ b/lisp/emacs-lisp/map.el @@ -5,7 +5,7 @@ ;; Author: Nicolas Petton ;; Maintainer: emacs-devel@gnu.org ;; Keywords: extensions, lisp -;; Version: 3.2 +;; Version: 3.2.1 ;; Package-Requires: ((emacs "26")) ;; This file is part of GNU Emacs. @@ -427,15 +427,15 @@ See `map-into' for all supported values of TYPE." "Convert MAP into a map of TYPE.") ;; FIXME: I wish there was a way to avoid this η-redex! -(cl-defmethod map-into (map (_type (eql 'list))) +(cl-defmethod map-into (map (_type (eql list))) "Convert MAP into an alist." (map-pairs map)) -(cl-defmethod map-into (map (_type (eql 'alist))) +(cl-defmethod map-into (map (_type (eql alist))) "Convert MAP into an alist." (map-pairs map)) -(cl-defmethod map-into (map (_type (eql 'plist))) +(cl-defmethod map-into (map (_type (eql plist))) "Convert MAP into a plist." (let (plist) (map-do (lambda (k v) (setq plist `(,v ,k ,@plist))) map) @@ -530,7 +530,7 @@ KEYWORD-ARGS are forwarded to `make-hash-table'." map) ht)) -(cl-defmethod map-into (map (_type (eql 'hash-table))) +(cl-defmethod map-into (map (_type (eql hash-table))) "Convert MAP into a hash-table with keys compared with `equal'." (map--into-hash map (list :size (map-length map) :test #'equal))) -- cgit v1.2.3