Subject: bug#43397: 28.0.50; Adding tool bar items: update tool bar

From: Stefan Monnier <[email-masked]>
Date: Tue, 03 May 2022 12:31:08 -0400

> Hm...  poking around in that code, I don't quite follow how this is all
> supposed to be hooked up, or which caches should be flushed.  Does the
> crystal ball have more insights?

This cache is broken:

    (defun tool-bar-make-keymap (&optional _ignore)
      "Generate an actual keymap from `tool-bar-map'.
    Its main job is to figure out which images to use based on the display's
    color capability and based on the available image libraries."
      (let ((key (cons (frame-terminal) tool-bar-map)))
        (or (gethash key tool-bar-keymap-cache)
            (puthash key (tool-bar-make-keymap-1) tool-bar-keymap-cache))))

`tool-bar-map` is a normal keymap, which we modify in the usual way,
i.e. via side-effect.  So the key we place in this `equal` hash table
will be routinely modified via side-effect, thus changing its sxhash.

Maybe we'd be better off using an `eq` hash table and manually flushing
the corresponding entry whenever `tool-bar-map` is modified by
side-effect.

I also see that we use a `:weakness t` but the values stored there will
usually not be stored anywhere else, so the hash table will be
completely flushed at every GC (and partly refilled soon after as part
of redisplay).  It should have `:weakness 'key` instead (but without
fixing the current bug report, this will cause the keymap to never be
refreshed until we manually flush the hash table ;-).

        Stefan