Subject: bug#42837: closed (Re: bug#42837: 26.3; `cl-incf' returns wrong value for (alist-get 'X ALIST 0) when ALIST is nil)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 13 Aug 2020 15:56:03 +0000

Your bug report

#42837: 26.3; `cl-incf' returns wrong value for (alist-get 'X ALIST 0) when ALIST is nil

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].

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

Subject: Re: bug#42837: 26.3; `cl-incf' returns wrong value for (alist-get 'X ALIST 0) when ALIST is nil
From: Stefan Kangas <[email-masked]>
To: Drew Adams <[email-masked]>
Cc: [email-masked]
Date: Thu, 13 Aug 2020 08:55:25 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Drew Adams <[email-masked]> writes:

> `cl-incf' should always return the new value of PLACE, after it is
> incremented.  It doesn't, if PLACE is an `alist-get' that returns the
> default value.
>
> (setq ali ())
>
> (cl-incf (alist-get 'a ali 0))
>
> returns ((a . 1)), but it should return just 1, the incremented value
> associated with key `a'.

I can reproduce this on Emacs 26.3 (under emacs -Q).  But it seems to
have been fixed on emacs-27 and current master, where I get:

(progn
   (require 'cl-lib)
   (setq ali ())
   (cl-incf (alist-get 'a ali 0)))

=> 1

I'm therefore closing this bug report.

Best regards,
Stefan Kangas

Subject: 26.3; `cl-incf' returns wrong value for (alist-get 'X ALIST 0) when ALIST is nil
From: Drew Adams <[email-masked]>
To: [email-masked]
Date: Wed, 12 Aug 2020 21:58:30 +0000 (UTC)
List-Id: <debbugs-submit.debbugs.gnu.org>

`cl-incf' should always return the new value of PLACE, after it is
incremented.  It doesn't, if PLACE is an `alist-get' that returns the
default value.

(setq ali ())

(cl-incf (alist-get 'a ali 0))

returns ((a . 1)), but it should return just 1, the incremented value 
associated with key `a'.

This is the definition of `cl-incf':

(defmacro cl-incf (place &optional x)
  "Increment PLACE by X (1 by default).
PLACE may be a symbol, or any generalized variable allowed by `setf'.
The return value is the incremented value of PLACE."
  (declare (debug (place &optional form)))
  (if (symbolp place)
      (list 'setq place (if x (list '+ place x) (list '1+ place)))
    (list 'cl-callf '+ place (or x 1))))

The definition should be something like this (not generally tested;
might need some fixes, more hygiene, or optimization):

(defmacro cl-incf (place &optional x)
  "Increment PLACE by X (1 by default).
PLACE may be a symbol, or any generalized variable allowed by `setf'.
The return value is the incremented value of PLACE."
  (declare (debug (place &optional form)))
  (if (symbolp place)
      (list 'setq place (if x (list '+ place x) (list '1+ place)))
    `(progn
       (cl-callf + ,place (or ,x 1))
       ,place)))

IOW, after updating the alist entry, `cl-incf' needs to return the new
value for the given key.  `cl-callf' currently returns the entire alist.

See this question from user `grepcake', who uncovered the problem:

https://emacs.stackexchange.com/q/60097/105

In GNU Emacs 26.3 (build 1, x86_64-w64-mingw32)
 of 2019-08-29
Repository revision: 96dd0196c28bc36779584e47fffcca433c9309cd
Windowing system distributor `Microsoft Corp.', version 10.0.18362
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''