bug#62119: 29.0.60; Add map-insert!, eventually get rid of map-not-inplace error

[Augusto Stoffel]

In my understanding, map.el should operate on values, like a functional
language, an not on objects (a.k.a. identities or places) like
procedural scripting languages.

This confusion is what originates the map-not-inplace error, which
should not exist at all, for the same reason that nconc, nreverse et
al. don't have this problem.

Also, this is not to say that map.el should not provide GV getters and
setters; it should.  But its functions should operate on values, even
the destructive ones.

So I suggest adding the following:

--8<---------------cut here---------------start------------->8---
(cl-defgeneric map-insert! (map key value)
  "Return a new map like MAP except that it associates KEY with VALUE.
This may destructively modify MAP to produce the new map."
  (map-insert map key value))

(cl-defmethod map-insert! ((map list) key value)
  (if (map--plist-p map)
      (if-let ((tail (memq key map)))
          (prog1 map
            (setf (cadr tail) value))
        `(,key ,value ,@map))
    (if-let ((pair (assoc key map)))
        (prog1 map
          (setf (cdr pair) value))
      `((,key . ,value) ,@map))))

(cl-defmethod map-insert! ((map hash-table) key value)
  (puthash key value map))
--8<---------------cut here---------------end--------------->8---

This gives:

  (map-insert! nil 'a 1)
  => ((a . 1))

  (map-insert! '((b . 2)) 'a 1)
  => ((a . 1) (b . 2))

While one the other hand:

  (map-put! nil 'a 1)
  => Debugger entered--Lisp error: (map-not-inplace nil)

  (map-put! '((b . 2)) 'a 1)
  => Debugger entered--Lisp error: (map-not-inplace ((b . 2)))

I would then suggest to make map-put! obsolete, due to its limitation
and conceptual confusion.  Also, the docstring could be clarified like
this:

--8<---------------cut here---------------start------------->8---
(cl-defgeneric map-put! (obj key value &optional testfn)
  "Associate KEY with VALUE in the map OBJ.
If KEY is already present in OBJ, replace the associated value
with VALUE.
This operates by modifying OBJ in place.  OBJ must be an object that
can be modified in place; otherwise signal a `map-not-inplace' error."
  ;; `testfn' only exists for backward compatibility with `map-put'!
  (declare (advertised-calling-convention (map key value) "27.1")))
--8<---------------cut here---------------end--------------->8---

As to the naming of map-insert!, we should first decide if the
exclamation mark is the convention we want for destructive operations.
One issue here is that map-delete is destructive.  In fact, there should
be a destructive and a non-destructive version of that method too.  In
theory no program should break if we changed map-delete to be
non-destructive (it currently can be so, but as usual there's no promise
of that).

As to my other recent tickets on map.el, I'm willing to provide the
patches once it's agreed what exactly should be done.