GNU bug report logs - #23105
25.0.92; map-put does not change its argument MAP

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23105 in the body.
You can then email your comments to 23105 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sho Takemori <stakemorii <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.92; map-put does not change its argument MAP
Date: Thu, 24 Mar 2016 21:09:31 +0900

[Message part 1 (text/plain, inline)]

I'm not sure if this is a bug, but the function map-put does not change its
argument MAP if the map is an alist and the key is not found.

For example, the following code returns "value" as expected:

(require 'map)
(let ((m (make-hash-table)))
  (map-put m 'key "value")
  (map-elt m 'key))

If I replace m by an alist, then the following code returns nil.
I expected it returned "value" as above.

(let ((m nil))
  (map-put m 'key "value")
  (map-elt m 'key))

By expanding map-put, I found that it returns a new list if "m" is a list
and
the key is not in "m".

Best regards,
Sho Takemori

In GNU Emacs 25.0.92.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.16.7)
 of 2016-03-24 built on K430-Ubuntu
Repository revision: c0165ea4d6ecf81db6728782c7322c311ee0a783
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
System Description: Ubuntu 15.10

Configured using:
 'configure --with-sound=no --with-file-notification=yes'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK GPM DBUS GCONF GSETTINGS NOTIFY
ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11

Important settings:
  value of $LC_MONETARY: ja_JP.UTF-8
  value of $LC_NUMERIC: ja_JP.UTF-8
  value of $LC_TIME: ja_JP.UTF-8
  value of $LANG: ja_JP.UTF-8
  value of $XMODIFIERS: @im=fcitx
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set
map
"value"
Mark set
nil [2 times]
scroll-up-command: End of buffer

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils map
seq byte-opt gv bytecomp byte-compile cconv cl-extra help-mode easymenu
cl-loaddefs pcase cl-lib time-date mule-util japan-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 95859 6137)
 (symbols 48 20522 0)
 (miscs 40 327 121)
 (strings 32 16995 4550)
 (string-bytes 1 505060)
 (vectors 16 13958)
 (vector-slots 8 527873 8024)
 (floats 8 182 21)
 (intervals 56 220 12)
 (buffers 976 11)
 (heap 1024 50485 944))

[Message part 2 (text/html, inline)]

Message #8 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Sho Takemori <stakemorii <at> gmail.com>
Cc: Nicolas Petton <nicolas <at> petton.fr>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Thu, 24 Mar 2016 16:57:01 +0100

Sho Takemori <stakemorii <at> gmail.com> writes:

> By expanding map-put, I found that it returns a new list if "m" is a
> list and the key is not in "m".

Indeed, good catch.

The implementation of `map-put' is wrong: it binds the evaluated place
expression to a new symbol and uses that symbol as place to operate on.

The intention presumably was to avoid code duplication in the macro
expansion - which is good - but it fails to change the original place.


Michael.

Message #11 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Sho Takemori <stakemorii <at> gmail.com>
Cc: 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Thu, 24 Mar 2016 17:39:30 +0100

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Sho Takemori <stakemorii <at> gmail.com> writes:
>
>> By expanding map-put, I found that it returns a new list if "m" is a
>> list and the key is not in "m".
>
> Indeed, good catch.
>
> The implementation of `map-put' is wrong: it binds the evaluated place
> expression to a new symbol and uses that symbol as place to operate on.
>
> The intention presumably was to avoid code duplication in the macro
> expansion - which is good - but it fails to change the original place.

Indeed, it is wrong.  I will have a look at the issue.

Nico

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Sho Takemori <stakemorii <at> gmail.com>
Cc: 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Thu, 24 Mar 2016 19:04:08 +0100

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Sho Takemori <stakemorii <at> gmail.com> writes:
>
>> By expanding map-put, I found that it returns a new list if "m" is a
>> list and the key is not in "m".
>
> Indeed, good catch.
>
> The implementation of `map-put' is wrong: it binds the evaluated place
> expression to a new symbol and uses that symbol as place to operate on.
>
> The intention presumably was to avoid code duplication in the macro
> expansion - which is good - but it fails to change the original place.

Now that I have a second look at it, I don't see why `map-put' (and
`map-delete' for that matter) are macros.  I'll see if I can replace
them with functions and have all the tests green.

Nico

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Sho Takemori <stakemorii <at> gmail.com>
Cc: 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Thu, 24 Mar 2016 19:26:44 +0100

[Message part 1 (text/plain, inline)]

Nicolas Petton <nicolas <at> petton.fr> writes:

> Now that I have a second look at it, I don't see why `map-put' (and
> `map-delete' for that matter) are macros.

Forget about that, I overlooked the issue.

Nico

[signature.asc (application/pgp-signature, inline)]

Message #24 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Sho Takemori <stakemorii <at> gmail.com>
To: Nicolas Petton <nicolas <at> petton.fr>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Sat, 26 Mar 2016 00:00:30 +0900

[Message part 1 (text/plain, inline)]

Thanks for the fix. But after the fix, if MAP is a list literal, then it
raises an error.
For example, this raises an error.
(map-put '((a . 1)) 'a "value")

This works.
(map-put [0 1 2] 1 "value")

It is an expected error?

Sho Takemori


2016-03-25 23:12 GMT+09:00 Nicolas Petton <nicolas <at> petton.fr>:

> Sho Takemori <stakemorii <at> gmail.com> writes:
>
> > I'm not sure if this is a bug, but the function map-put does not change
> its
> > argument MAP if the map is an alist and the key is not found.
>
> I pushed a fix to emacs-25.
>
> Nico
>

[Message part 2 (text/html, inline)]

Message #27 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: Sho Takemori <stakemorii <at> gmail.com>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Fri, 25 Mar 2016 16:14:43 +0100

[Message part 1 (text/plain, inline)]

Sho Takemori <stakemorii <at> gmail.com> writes:

> Thanks for the fix. But after the fix, if MAP is a list literal, then it
> raises an error.
> For example, this raises an error.
> (map-put '((a . 1)) 'a "value")
>
> This works.
> (map-put [0 1 2] 1 "value")
>
> It is an expected error?

Yes, in the sense that the same error will be raised if you try to do it
with a setf on an alist:

(setf (alist-get 'b '((a . 1))) 3) ;; => invalid-function (a . 1)

(let ((alist '((a . 1))))
  (setf (alist-get 'b alist) 3)) ;; => works fine

Note however that mutating literal lists is in general not a good idea,
as conses might be shared.

Nico

[signature.asc (application/pgp-signature, inline)]

Message #30 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Sho Takemori <stakemorii <at> gmail.com>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Sat, 26 Mar 2016 00:20:49 +0900

[Message part 1 (text/plain, inline)]

OK. Thanks for the reply.


2016-03-26 0:14 GMT+09:00 Nicolas Petton <nicolas <at> petton.fr>:

> Sho Takemori <stakemorii <at> gmail.com> writes:
>
> > Thanks for the fix. But after the fix, if MAP is a list literal, then it
> > raises an error.
> > For example, this raises an error.
> > (map-put '((a . 1)) 'a "value")
> >
> > This works.
> > (map-put [0 1 2] 1 "value")
> >
> > It is an expected error?
>
> Yes, in the sense that the same error will be raised if you try to do it
> with a setf on an alist:
>
> (setf (alist-get 'b '((a . 1))) 3) ;; => invalid-function (a . 1)
>
> (let ((alist '((a . 1))))
>   (setf (alist-get 'b alist) 3)) ;; => works fine
>
> Note however that mutating literal lists is in general not a good idea,
> as conses might be shared.
>
> Nico
>

[Message part 2 (text/html, inline)]

Message #33 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: Sho Takemori <stakemorii <at> gmail.com>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Fri, 25 Mar 2016 18:32:11 +0100

Nicolas Petton <nicolas <at> petton.fr> writes:

> > It is an expected error?
>
> Yes, in the sense that the same error will be raised if you try to do it
> with a setf on an alist:

> [...]

> Note however that mutating literal lists is in general not a good idea,
> as conses might be shared.

Calling `map-put' in this way is also not useful.


Michael.

Message #36 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Nicolas Petton <nicolas <at> petton.fr>
Cc: Sho Takemori <stakemorii <at> gmail.com>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Fri, 25 Mar 2016 18:44:13 +0100

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Calling `map-put' in this way is also not useful.

Maybe you could add to the docstring that `map-put' accepts a place
expression (or generalized variable).


Thanks,

Michael

Message #39 received at 23105 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Petton <nicolas <at> petton.fr>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Sho Takemori <stakemorii <at> gmail.com>, 23105 <at> debbugs.gnu.org
Subject: Re: bug#23105: 25.0.92; map-put does not change its argument MAP
Date: Fri, 25 Mar 2016 19:45:14 +0100

[Message part 1 (text/plain, inline)]

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
>> Calling `map-put' in this way is also not useful.
>
> Maybe you could add to the docstring that `map-put' accepts a place
> expression (or generalized variable).

Yes, I guess that'd be useful.

Nico

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.