Subject: bug#70524: closed (Re: bug#70524: [PATCH] Fix `map-elt` with `setf` for subplaces)

From: [email-masked] (GNU bug Tracking System)
Date: Mon, 06 May 2024 14:02:02 +0000

Your bug report

#70524: [PATCH] Fix `map-elt` with `setf` for subplaces

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
70524: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70524
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#70524: [PATCH] Fix `map-elt` with `setf` for subplaces
From: Michael Heerdegen <[email-masked]>
To: [email-masked]
Date: Mon, 06 May 2024 16:02:01 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

It seems the discussion has come to an end, and we have not decided to
do anything, so I'm closing this report for now.

Feel free to reopen when there is something new, or to continue
discussing other aspects in this or other/new bug reports.

Thank you,

Michael.

Subject: [PATCH] Fix `map-elt` with `setf` for subplaces
From: Okamsn <[email-masked]>
To: [email-masked]
Date: Tue, 23 Apr 2024 02:10:42 +0000
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

Currently, the use

     (let ((arr (vector 0 1 2 3 4 5 6)))
       (setf (map-elt (cl-subseq arr 3) 0)
             27)
       arr)

expands to

     (let ((arr (vector 0 1 2 3 4 5 6)))
       (let* ((v arr))
         (condition-case nil
             (with-no-warnings
               (map-put! (cl-subseq v 3) 0 27 nil))
           (map-not-inplace
            (let* ((new (map-insert (cl-subseq v 3) 0 27)))
              (progn
                (cl-replace v new :start1 3 :end1 nil)
                new))
            27)))
       arr)

which does not modify the original variable `arr` due to how `map-put!` 
is being used. With the attached patch, it would expand to

     (let ((arr (vector 0 1 2 3 4 5 6)))
       (let* ((v arr))
         (condition-case nil
             (with-no-warnings
               (let* ((m (cl-subseq v 3)))
                 (progn
                   (map-put! m 0 27 nil)
                   (let* ((new m))
                     (progn
                       (cl-replace v new :start1 3 :end1 nil)
                       new))
                   27)))
           (map-not-inplace
            (let* ((new (map-insert (cl-subseq v 3) 0 27)))
              (progn
                (cl-replace v new :start1 3 :end1 nil)
                new))
            27)))
       arr)

which correctly sets the value using `cl-replace` as the setter for 
`cl-subseq`.

Thank you.

From 1ac65858dc2f241975df8620ac79c973fcc6dad4 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <[email-masked]>
Date: Mon, 22 Apr 2024 21:45:03 -0400
Subject: [PATCH] Make setf with map-elt work with subplaces.

* lisp/emacs-lisp/map.el (map-elt): Using the setting function defined
by 'gv-letplace' to make sure that we capture the modification, even
when using 'map-put!'.  Otherwise, the sub-place might be understood as
a normal function call that will create the value to modify instead.

* test/lisp/emacs-lisp/map-tests.el (test-setf-map-with-function): Test
that sub-places work with 'map-elt' as expected.  For example, the use
of 'substring' should be understood as wanting to modify an existing
string, not wanting to modify the new string that would be created by a
call to 'substring'.
---
 lisp/emacs-lisp/map.el            | 6 +++++-
 test/lisp/emacs-lisp/map-tests.el | 5 +++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/map.el b/lisp/emacs-lisp/map.el
index d3d71a36ee4..facfdd8de7b 100644
--- a/lisp/emacs-lisp/map.el
+++ b/lisp/emacs-lisp/map.el
@@ -167,7 +167,11 @@ map-elt
                        `(condition-case nil
                             ;; Silence warnings about the hidden 4th arg.
                             (with-no-warnings
-                              (map-put! ,mgetter ,key ,v ,testfn))
+                              ,(macroexp-let2 nil m mgetter
+                                 `(progn
+                                    (map-put! ,m ,key ,v ,testfn)
+                                    ,(funcall msetter m)
+                                    ,v)))
                           (map-not-inplace
                            ,(funcall msetter
                                      `(map-insert ,mgetter ,key ,v))
diff --git a/test/lisp/emacs-lisp/map-tests.el b/test/lisp/emacs-lisp/map-tests.el
index dc8121b0582..c79bac54cb3 100644
--- a/test/lisp/emacs-lisp/map-tests.el
+++ b/test/lisp/emacs-lisp/map-tests.el
@@ -723,6 +723,11 @@ test-setf-map-with-function
     ;; Check that the function is only called once.
     (should (= num 1))))
 
+(ert-deftest test-setf-map-with-subplace ()
+  (let ((arr (string ?0 ?1 ?2 ?3 ?4 ?5 ?6)))
+    (setf (map-elt (substring arr 3) 0) ?x)
+    (should (equal arr (string ?0 ?1 ?2 ?x ?4 ?5 ?6)))))
+
 (ert-deftest test-map-plist-member ()
   "Test `map--plist-member' and `map--plist-member-1'."
   (dolist (mem '(map--plist-member map--plist-member-1))
-- 
2.34.1