From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "Basil L. Contovounesios" via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#47368: 28.0.50; map-elt returns nil without "deprecated" TESTFN Date: Wed, 15 Sep 2021 13:50:43 +0100 Message-ID: <87o88uroik.fsf@tcd.ie> References: <874kcnw0u0.fsf@web.de> <87a6m32pqn.fsf@web.de> <87a6kfkyom.fsf@web.de> <87wnnisy9c.fsf@tcd.ie> Reply-To: "Basil L. Contovounesios" Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28881"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Adam Porter , Nicolas Petton , 47368@debbugs.gnu.org To: Michael Heerdegen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Sep 15 14:51:41 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mQUO2-0007DZ-Ag for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 15 Sep 2021 14:51:38 +0200 Original-Received: from localhost ([::1]:56186 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mQUO1-00065a-9i for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 15 Sep 2021 08:51:37 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:42354) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mQUNU-0005zO-I3 for bug-gnu-emacs@gnu.org; Wed, 15 Sep 2021 08:51:06 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:39682) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mQUNS-00060I-QF for bug-gnu-emacs@gnu.org; Wed, 15 Sep 2021 08:51:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mQUNS-00014G-HP for bug-gnu-emacs@gnu.org; Wed, 15 Sep 2021 08:51:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Basil L. Contovounesios" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 15 Sep 2021 12:51:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47368 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: moreinfo Original-Received: via spool by 47368-submit@debbugs.gnu.org id=B47368.16317102584089 (code B ref 47368); Wed, 15 Sep 2021 12:51:02 +0000 Original-Received: (at 47368) by debbugs.gnu.org; 15 Sep 2021 12:50:58 +0000 Original-Received: from localhost ([127.0.0.1]:51228 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mQUNN-00013t-IF for submit@debbugs.gnu.org; Wed, 15 Sep 2021 08:50:58 -0400 Original-Received: from mail-wr1-f47.google.com ([209.85.221.47]:44650) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mQUNI-00013a-IF for 47368@debbugs.gnu.org; Wed, 15 Sep 2021 08:50:56 -0400 Original-Received: by mail-wr1-f47.google.com with SMTP id d6so3688905wrc.11 for <47368@debbugs.gnu.org>; Wed, 15 Sep 2021 05:50:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tcd.ie; s=google21; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=L2lljR/e/7O0197ZHnxPO24sJP9UfdLrIPNF85Cxx74=; b=LQv66Ah+xkcr2vebWa4Po2O2uDS/sdELAMkZOaJsEtdM0WBSoiu2DmAXKHBZ9/ALaE 0KpXViDtaQ4VvxlLCbxwjdE3HJuiK7MhRUPkhmg4Pa5m69kblhkjwqqZSuqoWVi1NHpz i2eSXhjuTNPJSwle2vP7/fP6UNi/tF3bPIGMK5SSFSQBtTss/p5BVEfvYY1AhPSf14cr iWYe/gLZIgyMzqujGHvbscKWXUQI1VhfKGJvesrb2gUqet996vTYCXH78u2jOK49n8OP 4DVN6sW575ZadWgt2TyiNJD5pFIR86d3SZw1yGqxRyhGfoXgsvY0Pwo81EnRJWgU+FK+ ur0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=L2lljR/e/7O0197ZHnxPO24sJP9UfdLrIPNF85Cxx74=; b=XaEmrndia+DawXRg0p8i1OGEsR7eZer2KtOhlNH6p2YM0vGzjnD9m9yk+mzY1WabTn hNAUVFPqZ2Yb/LXaoxdzIlmebRKVjdrvVmVrDcsjVZDQ73tXkVWjPxAZTkYQIbW5eRPU +xrKV6J1+53ZTjDMkg9hRtKrj2ziXI0rZyMmmqERXSgaCG234k/SWOfaV6mNtpXnzI6j nRcwUX3dNdRpE7QCBRXOKtLNdmNqdr6oRgnEclBbyPtuGRrG2RcHlihRRImApoyjiyfo 1Zpsk4zoQKc80o5kwniv5DGFQwzOiavLI7D7V6rG2sfcZ1in9i+dx/bcnjTapoGlgQ7T ofGw== X-Gm-Message-State: AOAM5327lsaI1uzFH/6eZq9gcDxNCygRFu9aqC3hRSMbvUlj9XgXLkAs 1TOHXZSQ+/sQeJnf2g4OyPWAAA== X-Google-Smtp-Source: ABdhPJw+JiuQH7U43qAj09jrFs+9TkOGwA8zb+cPOBNtmcwQMP2xZTKns11qjkVu2Z9NOyq02oqTGQ== X-Received: by 2002:a5d:6288:: with SMTP id k8mr4921945wru.137.1631710246220; Wed, 15 Sep 2021 05:50:46 -0700 (PDT) Original-Received: from localhost ([2a02:8084:20e2:c380:f410:82e8:3a21:eedf]) by smtp.gmail.com with ESMTPSA id u8sm4415936wmq.45.2021.09.15.05.50.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Sep 2021 05:50:45 -0700 (PDT) In-Reply-To: <87wnnisy9c.fsf@tcd.ie> (Basil L. Contovounesios's message of "Tue, 14 Sep 2021 21:22:39 +0100") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:214394 Archived-At: --=-=-= Content-Type: text/plain Basil L. Contovounesios [2021-09-14 21:22 +0100] wrote: > Michael Heerdegen [2021-09-14 16:40 +0200] wrote: > >> Adam Porter writes: >> >>> Friendly ping. :) The cutting of the Emacs 28 release branch >>> approaches, and I'd really like to have this fixed in Emacs 28. >> >> Basil, are you maybe able to care about this one? > > What was the conclusion? That map-elt should use equal for alists? > If so, sure, I'll try to propose a patch soon. How's the attached? Thanks, -- Basil --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Consistently-test-alist-keys-with-equal-in-map.el.patch >From a62bb01dd43a9957fe5f04552390a0b3e46e32f9 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" Date: Wed, 15 Sep 2021 00:17:26 +0100 Subject: [PATCH] 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(-) diff --git a/etc/NEWS b/etc/NEWS index 0c9dded458..b2ffbff2ef 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -3558,6 +3558,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 77431f0c59..e0af448eaf 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 @@ map--plist-p (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 @@ map-elt :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 @@ map-put 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--plist-delete (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 @@ map-delete ;; 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 c0f0dbc92b..afade8e295 100644 --- a/test/lisp/emacs-lisp/map-tests.el +++ b/test/lisp/emacs-lisp/map-tests.el @@ -85,11 +85,13 @@ test-map-elt-default (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 @@ test-map-put!-new-keys (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 @@ test-map-delete-empty (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))) -- 2.33.0 --=-=-=--