Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly

From: Helmut Eller <[email-masked]>
Date: Sat, 12 Jul 2025 11:05:36 +0200

On Sat, Jul 12 2025, Eli Zaretskii wrote:

>> Cc: [email-masked]
>> From: Helmut Eller <[email-masked]>
>> Date: Fri, 11 Jul 2025 23:10:15 +0200
>> 
>> A simple fix for Fminor_mode_key_binding would be to copy the modes and
>> maps arrays to an alloca'd area before calling Flookup_key.
>> 
>> In the patch below (for master) I tried something different: remove the
>> global variables cmm_maps and cmm_modes entirely.
>> 
>> Assuming that there are rarely more than a handful of minor modes
>> active, we can allocate the temporary arrays on the stack; for the case
>> with very many minor modes we use the GC heap.
>
> We usually don't use the stack for any data larger than 16KB.  Does
> this case fit that limit?

Yes: sizeof (struct cmm_vector) == 288.

>> The patch uses the GC heap if there are more than 16 minor mode keymaps
>> active.  I'm not sure if that is enough; viper mode already activates 8.
>> 
>> A nice aspect of the patch is that Fcurrent_active_maps no longer uses
>> the temparary arrays: the code in the patch knows how to build a list in
>> reverse order.  So with the patch, current_minor_maps is called less
>> frequently.  The most frequent callers are now tool_bar_items and
>> tab_bar_items which call current_minor_maps when the user scrolls
>> around.
>
> Thanks, but please also explain why we should install something like
> this on master and not on the igc branch, since the discussion started
> from the latter.

The main reason is that Pip said: "We should really fix this on master
and just use vectors..."

> What advantages will this get us on master?

On master, current_minor_maps returns a pointer to a buffer.  That
pointer is safe to use only until the next call to current_minor_maps.
This constraint is easy to forget: Fminor_mode_key_binding uses the
pointer for longer than that.

(Fminor_mode_key_binding calls Flookup_key which can execute arbitrary
Lisp code and hence could call current_minor_maps while the pointer to
the buffer is still used.).

With the patch, this bug would be fixed; that's only a tiny advantage
because in practice the problem occurs extremely rarely.

A more important advantage would be that it reduces "mental load"
because the new current_minor_maps is easier to use.

Helmut