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

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

[Drew Adams]

`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''

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

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> 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