From unknown Fri Sep 19 16:07:17 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#78943 <78943@debbugs.gnu.org> To: bug#78943 <78943@debbugs.gnu.org> Subject: Status: feature/igc [PATCH] Trace current minor maps exactly Reply-To: bug#78943 <78943@debbugs.gnu.org> Date: Fri, 19 Sep 2025 23:07:17 +0000 retitle 78943 feature/igc [PATCH] Trace current minor maps exactly reassign 78943 emacs submitter 78943 Helmut Eller severity 78943 normal tag 78943 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 02 08:28:59 2025 Received: (at submit) by debbugs.gnu.org; 2 Jul 2025 12:28:59 +0000 Received: from localhost ([127.0.0.1]:36373 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uWwaA-00048V-9C for submit@debbugs.gnu.org; Wed, 02 Jul 2025 08:28:59 -0400 Received: from lists.gnu.org ([2001:470:142::17]:48066) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uWwa7-00047H-Ew for submit@debbugs.gnu.org; Wed, 02 Jul 2025 08:28:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uWwa1-0004dV-Tj for bug-gnu-emacs@gnu.org; Wed, 02 Jul 2025 08:28:49 -0400 Received: from mail-ed1-x52c.google.com ([2a00:1450:4864:20::52c]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1uWwa0-00087u-1j for bug-gnu-emacs@gnu.org; Wed, 02 Jul 2025 08:28:49 -0400 Received: by mail-ed1-x52c.google.com with SMTP id 4fb4d7f45d1cf-606b58241c9so6972578a12.3 for ; Wed, 02 Jul 2025 05:28:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1751459326; x=1752064126; darn=gnu.org; h=mime-version:user-agent:message-id:date:subject:to:from:from:to:cc :subject:date:message-id:reply-to; bh=n3yZG1E/QUKaOS1e0fyQ9xojTVNHraCosAw2Y4zAQNk=; b=kDG3nbHWCLkt4YYLva1c9rJOfbl9/ogFdJZ3Afqi5Ww+7QOUFwzZCZFrO6hNQyGs34 GqAhO2f7/qff0/KiFvswDByHV+SxHy7oYrHAjhYeKqNKmNRL/NXZrT0e9hi/cz8Dpdmj NTKxzEn4qqw/LDJQa4X7lQaXjDHaQYxMpIf0F++G5RB0OYjkgmBxC+SgmF9RLkuUMnmX moGP2siZz0Z5UZ09BsNqeGCIDBxkHzQfb0HI4wLRj7Dbv72b6x1rIhA/sajBeK/1GyLc 2gY+ArAfcq+GzHAYzvLN6qG+ON01aUZY4WRPShHJ7MiIX6S+8JH+StT9gHvK+vlxTQCl QRmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1751459326; x=1752064126; h=mime-version:user-agent:message-id:date:subject:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=n3yZG1E/QUKaOS1e0fyQ9xojTVNHraCosAw2Y4zAQNk=; b=AjPhqZqiXeFhbxjvugP9UU0aslDCSWhKOZHT+paMrPPOdv4AU0pEtl89/1TU1KzLPe rT+iJK8I5RPY08GHuYZDLWx85IbDpyIdw1bkyOulXFlb7viEAZGDF6mtWhLSpEACD9QR NB49BoztzpC1ezehFHewToWLkBTNfm4njpKQNSuHcNm2ELB+y22WJsU4ZX8G6az6cLUK b5geGt8UoVa5OmujgVlGpskd9gBhIXfWwbauKKxycsG/0sKM1IBr83KQ8YeXJ0J+Z1He k1ZXg+USU+PdeM3GRolkIvvLUd/2OA0goLkodqGiAAM0yJpv8BWbBy0WHaEHSRh0rH0T PBKw== X-Gm-Message-State: AOJu0Yyi1sAoRMIkVFlIcRIrk0g8xNdf+CWhNwTT7BretYR+3RPK7SsK wDmtf9VAugsCUN0Vb3FAVFBKBv0/JnNZCS0nmRdyI1kqKTdbL5E6W19UC2Iu1g== X-Gm-Gg: ASbGnctdjRefM4MKerKvw/PuYM2qqarVIK3PHYjGJHm6VhaAacp2FHX8PufiSKUz39Z cb3XLsKWEllwzx9T321OXD8APWzExOiRixNK8RDf7hzosB1ioNTLSVTirNJ3x1g5atOS4wYkqwJ v2AA3wXIB2rvzKReS5yaIoqT8TqbhseisXKKoFAS46Cd9a7W/m+D/aCMWdIXDMnCrIF+qHJKYc7 0qzTr2Or/ePUnljlAiuqWSmp0Kkw92BiktgyEI2Ran6nRavtIWbmLpl73nD3WsHSlDjJ5CI/p4H q0NTfb9Qz7/vKaNUSc0TUADccziE3PkPwVrpLIdwV1Ka+1AgozIC7KUXwPi+vNwlYktxnQ== X-Google-Smtp-Source: AGHT+IGA0TXPb0VUsSAq5LLrGypcVaiznSAcmAiprq7fyRPob1B0FhJbg8oOA8otmapMZzZGf3HgXw== X-Received: by 2002:a17:907:7f2a:b0:ae3:696c:60a with SMTP id a640c23a62f3a-ae3c2a6c727mr257625266b.8.1751459325729; Wed, 02 Jul 2025 05:28:45 -0700 (PDT) Received: from caladan ([31.177.115.39]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae3b94b3b2esm235672966b.62.2025.07.02.05.28.45 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Jul 2025 05:28:45 -0700 (PDT) From: Helmut Eller To: bug-gnu-emacs@gnu.org Subject: feature/igc [PATCH] Trace current minor maps exactly X-Debbugs-Cc: Date: Wed, 02 Jul 2025 14:28:44 +0200 Message-ID: <87o6u2ixoj.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Received-SPF: pass client-ip=2a00:1450:4864:20::52c; envelope-from=eller.helmut@gmail.com; helo=mail-ed1-x52c.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 1.0 (+) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) --=-=-= Content-Type: text/plain Tags: patch --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Trace-current-minor-maps-exactly.patch >From 3a1a4549a0545afbddc96a87b404b6c7b835d5b7 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Wed, 2 Jul 2025 14:10:56 +0200 Subject: [PATCH] Trace current minor maps exactly * src/keymap.c (current_minor_maps): Use igc_xalloc_lisp_objs_exact instead of igc_xzalloc_ambig. --- src/keymap.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/keymap.c b/src/keymap.c index e5994216875..51bf4eb1d37 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -1573,7 +1573,7 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) if (i >= cmm_size) { - ptrdiff_t newsize, allocsize; + ptrdiff_t newsize; Lisp_Object *newmodes, *newmaps; /* Check for size calculation overflow. Other code @@ -1584,13 +1584,16 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) break; newsize = cmm_size == 0 ? 30 : cmm_size * 2; - allocsize = newsize * sizeof *newmodes; +#ifndef HAVE_MPS + ptrdiff_t allocsize = newsize * sizeof *newmodes; +#endif /* Use malloc here. See the comment above this function. Avoid realloc here; it causes spurious traps on GNU/Linux [KFS] */ block_input (); #ifdef HAVE_MPS - newmodes = igc_xzalloc_ambig (allocsize); + newmodes = igc_xalloc_lisp_objs_exact (newsize, + "current-minor-modes"); #else newmodes = malloc (allocsize); #endif @@ -1610,7 +1613,8 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) } #ifdef HAVE_MPS - newmaps = igc_xzalloc_ambig (allocsize); + newmaps = igc_xalloc_lisp_objs_exact (newsize, + "current-minor-maps"); #else newmaps = malloc (allocsize); #endif -- 2.39.5 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Wed Jul 09 10:29:45 2025 Received: (at 78943) by debbugs.gnu.org; 9 Jul 2025 14:29:45 +0000 Received: from localhost ([127.0.0.1]:51347 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uZVns-0000lQ-QC for submit@debbugs.gnu.org; Wed, 09 Jul 2025 10:29:45 -0400 Received: from mail-10629.protonmail.ch ([79.135.106.29]:24261) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uZVnp-0000jq-Ll for 78943@debbugs.gnu.org; Wed, 09 Jul 2025 10:29:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1752071374; x=1752330574; bh=O0wV9F1Q0Rrbg+5cLLu9243HOA3Jf2KNRqn+aNg8GhY=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=SeS6sNJXR1kfGjn+jtffB6eNCEyFIpQ0T9MLeOpeC5+h5f9RzDON9Du4c9EtWkiF6 FC8vl+P3s+nc++HBZoqU26TyWTZIeaFaLxCI1Uv5rahVDXs9JKrurj8w0mYxwnBdSH TFlB3ElHPDyGfDM3RPZrbAh39jrSGGgZ43lAQx7Tksx99tR7cXoMIkzoY8ngSUXVU+ zJRKxlgqx1ajpubifwJdP7aQKu2SsPTebbd6pxGqx4VPTyNFISp8A6cf5wqtmVV1ly ig1IMD1xjrb6oXD+RkdT+DUx3R4BP9kd88fIVLsACCm1pWYUfj7SAY8+uWpItbk2IR 0nGOtHaU7LLiw== Date: Wed, 09 Jul 2025 14:29:29 +0000 To: Helmut Eller From: Pip Cet Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly Message-ID: <87a55dsaii.fsf@protonmail.com> In-Reply-To: <87o6u2ixoj.fsf@gmail.com> References: <87o6u2ixoj.fsf@gmail.com> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: 4009c9e97a6e11e251952b45668af7e383f73a91 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) "Helmut Eller" writes: > From 3a1a4549a0545afbddc96a87b404b6c7b835d5b7 Mon Sep 17 00:00:00 2001 > From: Helmut Eller > Date: Wed, 2 Jul 2025 14:10:56 +0200 > Subject: [PATCH] Trace current minor maps exactly > > * src/keymap.c (current_minor_maps): Use igc_xalloc_lisp_objs_exact > instead of igc_xzalloc_ambig. Thanks, LGTM. Hope that applies to the other accumulated patches, too, then I'll install them all. This is a preexisting bug, but I'm suspicious about the code leaking a pointer to an igc_xzalloc'd area to its caller, which may call Lisp (when it autoloads a map) in the case of Fminor_mode_key_binding. If that Lisp code recursively accesses keymaps and the maps get reallocated, we end up resurrecting free'd data, and will probably crash. (My suspicion is this is the reason for the "don't use realloc" comment, too). We should really fix this on master and just use vectors, and it's probably even less effort for you to do so than it is for me, but I still don't get how code like this is meant to be fixed. (I hope the comment about static being defined to the empty string is no longer accurate :-) ). Pip From debbugs-submit-bounces@debbugs.gnu.org Fri Jul 11 17:10:29 2025 Received: (at 78943) by debbugs.gnu.org; 11 Jul 2025 21:10:29 +0000 Received: from localhost ([127.0.0.1]:40562 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uaL0l-0006Qb-6u for submit@debbugs.gnu.org; Fri, 11 Jul 2025 17:10:28 -0400 Received: from mail-ed1-x52d.google.com ([2a00:1450:4864:20::52d]:56674) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1uaL0h-0006Po-Tc for 78943@debbugs.gnu.org; Fri, 11 Jul 2025 17:10:25 -0400 Received: by mail-ed1-x52d.google.com with SMTP id 4fb4d7f45d1cf-60c9d8a169bso4220988a12.1 for <78943@debbugs.gnu.org>; Fri, 11 Jul 2025 14:10:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752268218; x=1752873018; darn=debbugs.gnu.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=c2hoX6DVvKjfKEUqjZbFjd7/oYsb6n8Tpy8h9W4qN80=; b=nVMp3axLhGa2eObyO4jeGZ800r+IDOdRzvi3vGNUerokXerUhhBgV2i+RzhOmBp6yJ wHyvknP5pX/Pue92sqeZb5QBZakcfJF8A0p6Wr4knqYoornK7K0KEzBfbnmbnCRbY9QN zIAWDTPl9d6OAx3mA/A8Qkl4gto8ZG5XmWmfcqFz5XbxUVqPhpWaYGoFrX6c8F31A5rd NxLP316gPoPLjUB/SNU+LMCej5WRumF/fdTw2T6rnasihWKoL7QrKIFXsvDFYGLABIwJ 695vVl4/HjeIgLzp7G6kmVw/W5pIvbopJmMjVJwjM/2TVDfIBCAOEJuUO6SdbU8Kh+17 OeXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752268218; x=1752873018; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=c2hoX6DVvKjfKEUqjZbFjd7/oYsb6n8Tpy8h9W4qN80=; b=Hr7Ordt/SMHvQAHpjWO7IWXirN5t1tLIu+2NeZevrWQq5crsaWfv97v4VXw8/C1jH9 SOqSUq5fjv5WIcMjZY9hKUxXQ6IOPSKA71rDfhR6ObJCEQ8kxpYcHhHh+2wpF+yS9h37 BPxkc9tM4Py9X5eJBUh79sOXK+h9PE3CGbKraDrzjTNAngw9teSNa6Ji24wFe1W8BTZY IPnuFqwRI/vVOoQ+eRq+L48JIDhPinSTjAUl97mGP3m+t/oZPuCaDjGlxgJpKoKEF6VA reTdDvill8UIQzvmn4PfKJxC6FBqFYCQ2+bvwyi6L1R/EeSfRRx7ZQwjxoSKH5PxsZuq jYbQ== X-Gm-Message-State: AOJu0Yws+w9wF5MkPlrRiuxNMN2A+qosiEbkRJ0WEwojvvgt3boit1Y1 CBv65u6hXOEKjK6oChB3q74MXTDm2NxoqPCrZed4dxm4NZk0We3mWdpkOHz4Qw== X-Gm-Gg: ASbGncvJoPSUMvqRB6GF4FpK9Uoi4MmHnZfen5tih+uGovaqaCmtFBLIXnQ1hJ3+7+s S6v8CwLpNaGYsDHK0YSqSbri0xAuaOtG32XWrEKI0VRfDTwiIlofQblO8d5DPOz9ytrZNg98p3p SV+TqeCDrKDpOkr9ZH5XNcL3ByZZzu/RPv82bT4EmkF21hf4jsTqZvygVfoD0s97XUauIx6HF/9 pkXO4IKRZ15pTUwud99MReztn1zp3DqLPO25YrY2yCLdlbRJr6kU4G3h4hfkMjR+Lx7l0SZSqVX WzYAoQNQCy/CH4VaNrFuzMm5d2CrXkEuICBL2FyUo2Aqsxn1ot2rZHpwXcZiSY2/5rZMqOam974 pJkU2RagM1h/dmqzTc9U= X-Google-Smtp-Source: AGHT+IHV3qg1pmXLZk5RvknA6TabgpDX2H/GBKd/2pqBBXp+BWiTTwDmFtYt/IpFvfsP8jLDHAuhzQ== X-Received: by 2002:a05:6402:2789:b0:608:523c:1365 with SMTP id 4fb4d7f45d1cf-611e84f3f0emr3960054a12.29.1752268217201; Fri, 11 Jul 2025 14:10:17 -0700 (PDT) Received: from caladan ([31.177.115.39]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-612067f7ca6sm387765a12.55.2025.07.11.14.10.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Jul 2025 14:10:16 -0700 (PDT) From: Helmut Eller To: Pip Cet Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: <87a55dsaii.fsf@protonmail.com> References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> Date: Fri, 11 Jul 2025 23:10:15 +0200 Message-ID: <8734b2h1s8.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain On Wed, Jul 09 2025, Pip Cet wrote: [...] > Thanks, LGTM. Hope that applies to the other accumulated patches, too, > then I'll install them all. Thanks. > This is a preexisting bug, but I'm suspicious about the code leaking a > pointer to an igc_xzalloc'd area to its caller, which may call Lisp > (when it autoloads a map) in the case of Fminor_mode_key_binding. If > that Lisp code recursively accesses keymaps and the maps get > reallocated, we end up resurrecting free'd data, and will probably > crash. (My suspicion is this is the reason for the "don't use realloc" > comment, too). > > We should really fix this on master and just use vectors, and it's > probably even less effort for you to do so than it is for me, but I > still don't get how code like this is meant to be fixed. > > (I hope the comment about static being defined to the empty string is no > longer accurate :-) ). :-) That's a curious comment. The overflow check in current_minor_maps is also curious. 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. 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. Helmut --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Eliminate-the-global-arrays-cmm_modes-and-cmm_maps.patch >From 4898642883085f335558a7e94387f0f1df20e5f7 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Fri, 11 Jul 2025 22:04:43 +0200 Subject: [PATCH] Eliminate the global arrays cmm_modes and cmm_maps The global variables are tricky to use, because the arrays can be reallocated. Use local variables instead; the new arrays are allocated on the stack or if they grow too large moved to the GC heap. * src/keymap.h (struct cmm_vector): New struct. (current_minor_maps): Use it. * src/keymap.c (traverse_minor_maps): New helper. Factored out from current_minor_maps. (current_minor_maps, current_minor_maps_2): Now implemented with traverse_minor_maps and cmm_vectors. (prepend_minor_mode_keymaps): New alternative to current_minor_maps that can be used by Fcurrent_active_maps and doesn't need a temporary buffer. (cmm_vector_init, check_cmm_vector, cmm_vector_grow, cmm_vector_push): New helpers for the cmm_vector type. (struct tconc, tconc_push, tconc_append): New helper struct. (collect_minor_mode_keymaps, collect_minor_mode_keymaps_2): New helpers. (cmm_modes, cmm_maps, cmm_size): Deleted. (Fcurrent_active_maps, Fcurrent_minor_mode_maps): Use prepend_minor_mode_keymaps. (Fminor_mode_key_binding, Fdescribe_buffer_bindings): Adapt to cmm_vectors. * src/keyboard.c (menu_bar_items, tab_bar_items, tool_bar_items): Adapt to cmm_vectors. --- src/keyboard.c | 22 ++-- src/keymap.c | 295 +++++++++++++++++++++++++++++-------------------- src/keymap.h | 21 +++- 3 files changed, 208 insertions(+), 130 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index b389d98feaa..679c340d499 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -8615,8 +8615,6 @@ menu_bar_items (Lisp_Object old) click and we need to switch buffers, we jump back here to rebuild the initial keymaps from the current buffer. */ { - Lisp_Object *tmaps; - /* Should overriding-terminal-local-map and overriding-local-map apply? */ if (!NILP (Voverriding_local_map_menu_flag) && !NILP (Voverriding_local_map)) @@ -8636,7 +8634,9 @@ menu_bar_items (Lisp_Object old) properties may not work reliable, as they are only recognized when the menu-bar (or mode-line) is updated, which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); + struct cmm_vector v; + current_minor_maps (&v); + ptrdiff_t nminor = v.len; SAFE_NALLOCA (maps, 1, nminor + 4); nmaps = 0; Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); @@ -8646,7 +8646,7 @@ menu_bar_items (Lisp_Object old) maps[nmaps++] = tem; if (nminor != 0) { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); + memcpy (maps + nmaps, v.maps, nminor * sizeof (maps[0])); nmaps += nminor; } maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); @@ -9152,7 +9152,6 @@ tab_bar_items (Lisp_Object reuse, int *nitems) Lisp_Object mapsbuf[3]; ptrdiff_t nmaps, i; Lisp_Object oquit; - Lisp_Object *tmaps; USE_SAFE_ALLOCA; *nitems = 0; @@ -9191,7 +9190,9 @@ tab_bar_items (Lisp_Object reuse, int *nitems) properties may not work reliably, as they are only recognized when the tab-bar (or mode-line) is updated, which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); + struct cmm_vector v; + current_minor_maps (&v); + ptrdiff_t nminor = v.len; SAFE_NALLOCA (maps, 1, nminor + 4); nmaps = 0; Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); @@ -9201,7 +9202,7 @@ tab_bar_items (Lisp_Object reuse, int *nitems) maps[nmaps++] = tem; if (nminor != 0) { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); + memcpy (maps + nmaps, v.maps, nminor * sizeof (maps[0])); nmaps += nminor; } maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); @@ -9536,7 +9537,6 @@ tool_bar_items (Lisp_Object reuse, int *nitems) Lisp_Object mapsbuf[3]; ptrdiff_t nmaps, i; Lisp_Object oquit; - Lisp_Object *tmaps; USE_SAFE_ALLOCA; *nitems = 0; @@ -9575,7 +9575,9 @@ tool_bar_items (Lisp_Object reuse, int *nitems) properties may not work reliably, as they are only recognized when the tool-bar (or mode-line) is updated, which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); + struct cmm_vector v; + current_minor_maps (&v); + ptrdiff_t nminor = v.len; SAFE_NALLOCA (maps, 1, nminor + 4); nmaps = 0; Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); @@ -9585,7 +9587,7 @@ tool_bar_items (Lisp_Object reuse, int *nitems) maps[nmaps++] = tem; if (nminor != 0) { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); + memcpy (maps + nmaps, v.maps, nminor * sizeof (maps[0])); nmaps += nminor; } maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); diff --git a/src/keymap.c b/src/keymap.c index 2c250578b00..f6e2e5bf6fb 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -1504,34 +1504,15 @@ silly_event_symbol_error (Lisp_Object c) /* Global, local, and minor mode keymap stuff. */ -/* We can't put these variables inside current_minor_maps, since under - some systems, static gets macro-defined to be the empty string. - Ickypoo. */ -static Lisp_Object *cmm_modes = NULL, *cmm_maps = NULL; -static ptrdiff_t cmm_size = 0; - -/* Store a pointer to an array of the currently active minor modes in - *modeptr, a pointer to an array of the keymaps of the currently - active minor modes in *mapptr, and return the number of maps - *mapptr contains. - - This function always returns a pointer to the same buffer, and may - free or reallocate it, so if you want to keep it for a long time or - hand it out to lisp code, copy it. This procedure will be called - for every key sequence read, so the nice lispy approach (return a - new assoclist, list, what have you) for each invocation would - result in a lot of consing over time. - - If we used xrealloc/xmalloc and ran out of memory, they would throw - back to the command loop, which would try to read a key sequence, - which would call this function again, resulting in an infinite - loop. Instead, we'll use realloc/malloc and silently truncate the - list, let the key sequence be read, and hope some other piece of - code signals the error. */ -ptrdiff_t -current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) -{ - ptrdiff_t i = 0; +/* Traverse the currently active minor modes, calling F with the + (minor-mode, keymap) pairs. Abort the traversal early, iff F + returns false. + */ +static void +traverse_minor_maps (bool (*f) (Lisp_Object mode, Lisp_Object map, + void *closure), + void *closure) +{ Lisp_Object alist, assoc, var, val; Lisp_Object emulation_alists = Vemulation_mode_map_alists; Lisp_Object lists[2]; @@ -1552,17 +1533,16 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) else alist = lists[list_number]; - for ( ; CONSP (alist); alist = XCDR (alist)) + for (; CONSP (alist); alist = XCDR (alist)) if ((assoc = XCAR (alist), CONSP (assoc)) && (var = XCAR (assoc), SYMBOLP (var)) - && (val = find_symbol_value (var), !BASE_EQ (val, Qunbound)) + && (val = find_symbol_value (var), + !BASE_EQ (val, Qunbound)) && !NILP (val)) { - Lisp_Object temp; - - /* If a variable has an entry in Vminor_mode_overriding_map_alist, - and also an entry in Vminor_mode_map_alist, - ignore the latter. */ + /* If a variable has an entry in + Vminor_mode_overriding_map_alist, and also an entry in + Vminor_mode_map_alist, ignore the latter. */ if (list_number == 1) { val = assq_no_quit (var, lists[0]); @@ -1570,68 +1550,151 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) continue; } - if (i >= cmm_size) - { - ptrdiff_t newsize, allocsize; - Lisp_Object *newmodes, *newmaps; - - /* Check for size calculation overflow. Other code - (e.g., read_key_sequence) adds 3 to the count - later, so subtract 3 from the limit here. */ - if (min (PTRDIFF_MAX, SIZE_MAX) / (2 * sizeof *newmodes) - 3 - < cmm_size) - break; - - newsize = cmm_size == 0 ? 30 : cmm_size * 2; - allocsize = newsize * sizeof *newmodes; - - /* Use malloc here. See the comment above this function. - Avoid realloc here; it causes spurious traps on GNU/Linux [KFS] */ - block_input (); - newmodes = malloc (allocsize); - if (newmodes) - { - if (cmm_modes) - { - memcpy (newmodes, cmm_modes, - cmm_size * sizeof cmm_modes[0]); - free (cmm_modes); - } - cmm_modes = newmodes; - } + /* Get the keymap definition--or nil if it is not + defined. */ + Lisp_Object map = Findirect_function (XCDR (assoc), Qt); + if (NILP (map)) + continue; + if (!f (var, map, closure)) + return; + } + } +} - newmaps = malloc (allocsize); - if (newmaps) - { - if (cmm_maps) - { - memcpy (newmaps, cmm_maps, - cmm_size * sizeof cmm_maps[0]); - free (cmm_maps); - } - cmm_maps = newmaps; - } - unblock_input (); +static void +cmm_vector_init (struct cmm_vector *v) +{ + v->len = 0; + v->capacity = ARRAYELTS (v->u.onstack) / 2; + v->modes = &v->u.onstack[0]; + v->maps = &v->u.onstack[v->capacity]; +} - if (newmodes == NULL || newmaps == NULL) - break; - cmm_size = newsize; - } +static bool +check_cmm_vector (struct cmm_vector *v) +{ + if (v->len > v->capacity) + return false; + bool onstack = v->capacity * 2 == ARRAYELTS (v->u.onstack); + bool onheap = (!onstack && VECTORP (v->u.onheap) + && v->capacity * 2 == ASIZE (v->u.onheap)); + if (onstack != !onheap) + return false; + Lisp_Object *contents + = (onstack ? v->u.onstack : XVECTOR (v->u.onheap)->contents); + return (v->modes == &contents[0] + && v->maps == &contents[v->capacity]); +} - /* Get the keymap definition--or nil if it is not defined. */ - temp = Findirect_function (XCDR (assoc), Qt); - if (!NILP (temp)) - { - cmm_modes[i] = var; - cmm_maps [i] = temp; - i++; - } - } +static void +cmm_vector_grow (struct cmm_vector *v) +{ + eassert (check_cmm_vector (v)); + size_t len = v->len; + size_t new_capacity = 2 * len; + Lisp_Object onheap = make_nil_vector (2 * new_capacity); + Lisp_Object *new_modes = aref_addr (onheap, 0); + Lisp_Object *new_maps = aref_addr (onheap, new_capacity); + for (size_t i = 0; i < len; i++) + { + new_modes[i] = v->modes[i]; + new_maps[i] = v->maps[i]; } + v->modes = new_modes; + v->maps = new_maps; + v->u.onheap = onheap; + v->capacity = new_capacity; +} + +static void +cmm_vector_push (struct cmm_vector *v, Lisp_Object mode, + Lisp_Object map) +{ + eassert (check_cmm_vector (v)); + size_t len = v->len; + if (len == v->capacity) + cmm_vector_grow (v); + v->modes[len] = mode; + v->maps[len] = map; + v->len = len + 1; +} + +static bool +current_minor_maps_2 (Lisp_Object mode, Lisp_Object map, + void *closure) +{ + struct cmm_vector *v = closure; + cmm_vector_push (v, mode, map); + return true; +} + +/* Initialize V, then push the currently active minor modes and + corresponding keymaps to V. + + At the end, V->mode points to an array of the currently active + minor modes, V->maps points to an array of the keymaps of the + currently active minor modes, and V->len is the length of those + arrays. +*/ +void +current_minor_maps (struct cmm_vector *v) +{ + cmm_vector_init (v); + traverse_minor_maps (current_minor_maps_2, v); +} + +/* Used for "tail concatenation". */ +struct tconc +{ + Lisp_Object head, tail; /* head is nil, if the list is empty */ +}; + +/* Append an element at the end. */ +static void +tconc_push (struct tconc *l, Lisp_Object item) +{ + Lisp_Object tail = list1 (item); + if (NILP (l->head)) + l->head = tail; + else + XSETCDR (l->tail, tail); + l->tail = tail; +} - if (modeptr) *modeptr = cmm_modes; - if (mapptr) *mapptr = cmm_maps; - return i; +/* Concatenate L and LIST and return the result as a list. */ +static Lisp_Object +tconc_append (struct tconc l, Lisp_Object list) +{ + if (NILP (l.head)) + return list; + XSETCDR (l.tail, list); + return l.head; +} + +static bool +collect_minor_mode_keymaps_2 (Lisp_Object mode, Lisp_Object map, + void *closure) +{ + struct tconc *l = closure; + eassert (!NILP (map)); + tconc_push (l, map); + return true; +} + +/* Build a list of the currently active minor mode keymaps. */ +static struct tconc +collect_minor_mode_keymaps (void) +{ + struct tconc list = { Qnil, Qnil }; + traverse_minor_maps (collect_minor_mode_keymaps_2, &list); + return list; +} + +static Lisp_Object +prepend_minor_mode_keymaps (Lisp_Object keymaps) +{ + struct tconc l = collect_minor_mode_keymaps (); + return tconc_append (l, keymaps); } /* Return the offset of POSITION, a click position, in the style of @@ -1704,8 +1767,6 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, if (NILP (XCDR (keymaps))) { - Lisp_Object *maps; - int nmaps; ptrdiff_t pt = click_position (position); /* This usually returns the buffer's local map, but that can be overridden by a `local-map' property. */ @@ -1775,11 +1836,7 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, keymaps = Fcons (local_map, keymaps); /* Now put all the minor mode keymaps on the list. */ - nmaps = current_minor_maps (0, &maps); - - for (int i = --nmaps; i >= 0; i--) - if (!NILP (maps[i])) - keymaps = Fcons (maps[i], keymaps); + keymaps = prepend_minor_mode_keymaps (keymaps); if (!NILP (keymap)) keymaps = Fcons (keymap, keymaps); @@ -1876,23 +1933,24 @@ DEFUN ("minor-mode-key-binding", Fminor_mode_key_binding, Sminor_mode_key_bindin bindings; see the description of `lookup-key' for more details about this. */) (Lisp_Object key, Lisp_Object accept_default) { - Lisp_Object *modes, *maps; - int nmaps = current_minor_maps (&modes, &maps); + struct cmm_vector v; + current_minor_maps (&v); + int nmaps = v.len; Lisp_Object binding = Qnil; int j; for (int i = j = 0; i < nmaps; i++) - if (!NILP (maps[i]) - && !NILP (binding = Flookup_key (maps[i], key, accept_default)) + if (!NILP (v.maps[i]) + && !NILP (binding = Flookup_key (v.maps[i], key, accept_default)) && !FIXNUMP (binding)) { if (KEYMAPP (binding)) - maps[j++] = Fcons (modes[i], binding); + v.maps[j++] = Fcons (v.modes[i], binding); else if (j == 0) - return list1 (Fcons (modes[i], binding)); + return list1 (Fcons (v.modes[i], binding)); } - return Flist (j, maps); + return Flist (j, v.maps); } DEFUN ("use-global-map", Fuse_global_map, Suse_global_map, 1, 1, 0, @@ -1937,10 +1995,7 @@ DEFUN ("current-minor-mode-maps", Fcurrent_minor_mode_maps, Scurrent_minor_mode_ doc: /* Return a list of keymaps for the minor modes of the current buffer. */) (void) { - Lisp_Object *maps; - int nmaps = current_minor_maps (0, &maps); - - return Flist (nmaps, maps); + return prepend_minor_mode_keymaps (Qnil); } /* Help functions for describing and documenting keymaps. */ @@ -2929,13 +2984,15 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b else { /* Print the minor mode and major mode keymaps. */ - Lisp_Object *modes, *maps; /* Temporarily switch to `buffer', so that we can get that buffer's minor modes correctly. */ Fset_buffer (buffer); - int nmaps = current_minor_maps (&modes, &maps); + struct cmm_vector v; + current_minor_maps (&v); + int nmaps = v.len; + Fset_buffer (outbuf); start1 = get_local_map (BUF_PT (XBUFFER (buffer)), @@ -2958,17 +3015,17 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b because it takes care of other features when doing so. */ char *title, *p; - if (!SYMBOLP (modes[i])) + if (!SYMBOLP (v.modes[i])) emacs_abort (); USE_SAFE_ALLOCA; - p = title = SAFE_ALLOCA (42 + SBYTES (SYMBOL_NAME (modes[i]))); + p = title = SAFE_ALLOCA (42 + SBYTES (SYMBOL_NAME (v.modes[i]))); *p++ = '\f'; *p++ = '\n'; *p++ = '`'; - memcpy (p, SDATA (SYMBOL_NAME (modes[i])), - SBYTES (SYMBOL_NAME (modes[i]))); - p += SBYTES (SYMBOL_NAME (modes[i])); + memcpy (p, SDATA (SYMBOL_NAME (v.modes[i])), + SBYTES (SYMBOL_NAME (v.modes[i]))); + p += SBYTES (SYMBOL_NAME (v.modes[i])); *p++ = '\''; memcpy (p, " Minor Mode Bindings", strlen (" Minor Mode Bindings")); p += strlen (" Minor Mode Bindings"); @@ -2976,9 +3033,9 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b Lisp_Object msg = build_unibyte_string (title); calln (Qhelp__describe_map_tree, - maps[i], Qt, shadow, prefix, + v.maps[i], Qt, shadow, prefix, msg, nomenu, Qnil, Qnil, Qnil, buffer); - shadow = Fcons (maps[i], shadow); + shadow = Fcons (v.maps[i], shadow); SAFE_FREE (); } diff --git a/src/keymap.h b/src/keymap.h index c9b47ffccd6..fcb64dc1853 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -32,11 +32,30 @@ #define KEYMAP_H #define KEY_DESCRIPTION_SIZE ((2 * 6) + 1 + (CHARACTERBITS / 3) + 1 + 1) #define KEYMAPP(m) (!NILP (get_keymap (m, false, false))) + +/* A collection of the currently active minor modes and keymaps. + + Typical use: + + struct cmm_vector v; + current_minor_maps (&v); + ... do something with v ... + */ +struct cmm_vector +{ + size_t len, capacity; + Lisp_Object *modes, *maps; /* those point into onstack or onheap */ + union { + Lisp_Object onstack[32]; + volatile Lisp_Object onheap; /* A Lisp_Vector. Pinned. */ + } u; +}; + extern Lisp_Object current_global_map; extern char *push_key_description (EMACS_INT, char *); extern Lisp_Object access_keymap (Lisp_Object, Lisp_Object, bool, bool, bool); extern Lisp_Object get_keymap (Lisp_Object, bool, bool); -extern ptrdiff_t current_minor_maps (Lisp_Object **, Lisp_Object **); +extern void current_minor_maps (struct cmm_vector *); extern void initial_define_lispy_key (Lisp_Object, const char *, const char *); extern void syms_of_keymap (void); -- 2.39.5 --=-=-=-- From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 12 02:20:16 2025 Received: (at 78943) by debbugs.gnu.org; 12 Jul 2025 06:20:16 +0000 Received: from localhost ([127.0.0.1]:43020 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uaTap-0003Kk-VB for submit@debbugs.gnu.org; Sat, 12 Jul 2025 02:20:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:41110) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uaTao-0003IJ-A2 for 78943@debbugs.gnu.org; Sat, 12 Jul 2025 02:20:14 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uaTaf-0005V8-My; Sat, 12 Jul 2025 02:20:07 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=RVhBXAQO8dq5BrBPrjyfomaGp1rtAQ0QZRJjaHCceQg=; b=FoZHdHsSBVfI I/rPhaaeNFzuolDAfWlBov0BWvrnVtTULFQlmxu80O6U5YW5MR8Tl8Xdq7qDLT0t04iVIpg96oQoO BaJtQ1IFcI83OKekVJvp1TaaEX6GqNMo/EuelInyzWsrF/eRboaToF/yzW8fqXbdNgEjnqybmpR9u knuKnHLMmNc3yme12wm27OCMGuqKaQF7zUbmk/cx7pn5UecLU/jTY7KtoDUXEsHcB2R774BEjI74U 4pb6jtXKlzkkePOzapp9fvPepGV+nAioWzAp7mZndrs2cxG7mLhelOmxPtbXZLpozmbYuCIBTzxTh uDuxLF84p4QFE3flOD2WFQ==; Date: Sat, 12 Jul 2025 09:19:59 +0300 Message-Id: <86v7nyexrk.fsf@gnu.org> From: Eli Zaretskii To: Helmut Eller , Stefan Monnier In-Reply-To: <8734b2h1s8.fsf@gmail.com> (message from Helmut Eller on Fri, 11 Jul 2025 23:10:15 +0200) Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78943 Cc: pipcet@protonmail.com, 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Cc: 78943@debbugs.gnu.org > From: Helmut Eller > 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? > 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. What advantages will this get us on master? Adding Stefan to the discussion. From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 12 05:05:46 2025 Received: (at 78943) by debbugs.gnu.org; 12 Jul 2025 09:05:46 +0000 Received: from localhost ([127.0.0.1]:43825 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uaWB0-00065C-9X for submit@debbugs.gnu.org; Sat, 12 Jul 2025 05:05:46 -0400 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]:45445) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1uaWAy-00064f-Eh for 78943@debbugs.gnu.org; Sat, 12 Jul 2025 05:05:45 -0400 Received: by mail-ej1-x632.google.com with SMTP id a640c23a62f3a-ae361e8ec32so512682566b.3 for <78943@debbugs.gnu.org>; Sat, 12 Jul 2025 02:05:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752311138; x=1752915938; darn=debbugs.gnu.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=36G5TbrE8TGGFzF5Jeb5+ITR3NCYCA9HmDWQhHGb9Eg=; b=O5U9412699UT0KB0hM4Wyh49FmFCf2+wVIfW0gSMsK70Ztqzx5UQeaGSbX45IFIDI+ KKV3FXs0Nhn83ykXTr1KVZ7KYoyx7qh51ubbLPDA9EEfAHzyFt+6Rf0CFD0z5TQNsIUh hISJiMGgUevq8zoho+ipRdJjCi9+n4HJ8y7vuqK+UfVlgsiWLfRODJwMmMe39A5ttiVn sfGOlbV50tETatsGdBQNdpaSxZBLxeKEkNNY5Xx54CnJcd3RKpSA6vYSAhYFg+ov6lI9 3q+PZREcIZ2IHB3v3Rb+Oo+XWkxUzY4V6C+K555Q/IxB1pJ8dkSapGUtSn9RpGcKX3gG tlpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752311138; x=1752915938; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=36G5TbrE8TGGFzF5Jeb5+ITR3NCYCA9HmDWQhHGb9Eg=; b=bQnZJ/DDxATwmoUOLkdbJFTubqRUIM50zKMyQdXe4fOgNISCg6uWM4sDM5GP+odIWO grpug2i18sNMM4ZqlusPVD5c8rEylqYxQM/6ZyXDI1biw2KZUSvUjLYIbj7BQtSkCIWb Jx1ycl4MDUlykc8GzOwY8J2/2tRTbHGJmJQA+qGY5ru+p6s+VOcMrRtoLdFgT1e6X2HM swkGGwasjyo5HJSf19H0FpirHKkNTpcRlAblbN/YG90N81RWVVoP+urmOA1HzZBP5rIB cZECJ2fYKANiBeuRTWfOXIQ+L8DZqlxLmzQNxlSh5u7HseDjVJRAdoeu2/x4HnUkzEE2 +B5Q== X-Forwarded-Encrypted: i=1; AJvYcCXsCHe13/TVza+AhEk7dgQNh0xd+yk9yuCXqbJ4ozQtERAaFqpPvzjSMNb2jdcBOS/pzVBOtw==@debbugs.gnu.org X-Gm-Message-State: AOJu0YypXDvVz13fgDBoacORogtqf58stk7BkA8Xzoos6lmWEvEM1k7Q hRK1ViNaY2gvx/v+19S2/d+DM1mlbL2Zf7Ys5jK9VRxG+zGgZVqfSob5NNOGVA== X-Gm-Gg: ASbGnctgmjv2XK8Y/6iyIgd/lQudwjyHlYjEy14b0XSlj8uicFSggOF+dLYXKvPiv/7 UEEojPB6qqC02r5xZ1m4/PMzLiMEdry+PTaVmGzeWfU+0VCpA08w+XLZ8p4eOAGhox36FtgVVLx WzhskiPe7Gc4z9ICFDvE79kW8HzWoh8BDs+sXSfYlhDavXTHA90Bwb5v0NwiZ15H/5MI4h0DFn4 CeA2ClA8WEFzMuYt5KZlkizQxbGy67n9PlGCS/aC+NgLzA/0qXtaWq/Ziac5M5BnSzCiB+QWtWO NfDfj+Gt7iiCPUWIlKr7uUtnYAJ5zwCHoG33/3huO1zAAucvYa3fp/qkFXg0hjZtO2Scadp+/Dq l8ZfJchHjspSs68NZeJs= X-Google-Smtp-Source: AGHT+IHt1cnGwfMhdLNZ5u8EdUVGCO3Z4UHnhZzmMnTUByMYEUqSZvKW3GTOwIAWobui5XmS1GzA1Q== X-Received: by 2002:a17:906:478a:b0:ae0:ad5c:4185 with SMTP id a640c23a62f3a-ae70131bae6mr532512566b.57.1752311137947; Sat, 12 Jul 2025 02:05:37 -0700 (PDT) Received: from caladan ([31.177.115.39]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae6e82df594sm447981566b.159.2025.07.12.02.05.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 12 Jul 2025 02:05:37 -0700 (PDT) From: Helmut Eller To: Eli Zaretskii Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: <86v7nyexrk.fsf@gnu.org> References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <86v7nyexrk.fsf@gnu.org> Date: Sat, 12 Jul 2025 11:05:36 +0200 Message-ID: <87v7nxg4nz.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: pipcet@protonmail.com, Stefan Monnier , 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) On Sat, Jul 12 2025, Eli Zaretskii wrote: >> Cc: 78943@debbugs.gnu.org >> From: Helmut Eller >> 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 From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 12 06:17:41 2025 Received: (at 78943) by debbugs.gnu.org; 12 Jul 2025 10:17:41 +0000 Received: from localhost ([127.0.0.1]:44292 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uaXIb-00031c-C3 for submit@debbugs.gnu.org; Sat, 12 Jul 2025 06:17:41 -0400 Received: from mail-24416.protonmail.ch ([109.224.244.16]:10335) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uaXIY-000315-6h for 78943@debbugs.gnu.org; Sat, 12 Jul 2025 06:17:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1752315451; x=1752574651; bh=oaH7/K2/bLn6fCTsx8/SqqpQfVUsIIRNfib33b4W90w=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=uw28VizePDPBANwCNqVobcRpLaWD4FIWtj9MvgHN8eEAkE+kocAGJExjyhexFcs/+ 52Csd0csoj1j1hmfmpquVIRoQMu5dhzfhXOSl+wRTcPy/+gg82god7UuoKowf1YSAS yCmR4PQ8X58O+yu8ml9Sg637KOGXW4fJq36GHfJWaKB931ypRdcokeFBND5H6kHO4z oMBTVOGbwLpPCzis1M84lndKdtvMmPJRGOEdo6KmtooxIESxKSPVuJmwrOVj2dofQ2 4TPAJWND79kkgMYQGNXnKm6b931OzO/JLZQrAYtRIeasP8y/y4nWKyOow7p0OKPo2R eT7Cv3yscldZg== Date: Sat, 12 Jul 2025 10:17:27 +0000 To: Helmut Eller From: Pip Cet Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly Message-ID: <87jz4dpvbh.fsf@protonmail.com> In-Reply-To: <8734b2h1s8.fsf@gmail.com> References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> Feedback-ID: 112775352:user:proton X-Pm-Message-ID: cec9392a6ce3b1a69c8c0d5ffe2f1dd919d19c99 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) "Helmut Eller" writes: > On Wed, Jul 09 2025, Pip Cet wrote: > > [...] >> Thanks, LGTM. Hope that applies to the other accumulated patches, too, >> then I'll install them all. > > Thanks. > >> This is a preexisting bug, but I'm suspicious about the code leaking a >> pointer to an igc_xzalloc'd area to its caller, which may call Lisp >> (when it autoloads a map) in the case of Fminor_mode_key_binding. If >> that Lisp code recursively accesses keymaps and the maps get >> reallocated, we end up resurrecting free'd data, and will probably >> crash. (My suspicion is this is the reason for the "don't use realloc" >> comment, too). >> >> We should really fix this on master and just use vectors, and it's >> probably even less effort for you to do so than it is for me, but I >> still don't get how code like this is meant to be fixed. >> >> (I hope the comment about static being defined to the empty string is no >> longer accurate :-) ). > > :-) That's a curious comment. The overflow check in current_minor_maps > is also curious. > > 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. Thanks. It'd be nice to have a more general approach to growable vectors, but in this case, I don't understand why we use array structures at all: two simple linked lists should work just as well. So I don't think a specific new structure for the cmm case is the right approach here; it should be easy enough to generalize to vectors of Lisp_Object, and the code would be cleaner without the "two vectors placed back-to-back" assumption. One place which would benefit from growable vectors is json.c. My idea still would be to grow a vector aggressively until it's large enough to fit all elements, then "truncate" it in place in such a way that the GC knows to shrink it when it gets copied to the next generation. However, even that approach may be too complicated. Pip From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 12 10:04:15 2025 Received: (at 78943) by debbugs.gnu.org; 12 Jul 2025 14:04:16 +0000 Received: from localhost ([127.0.0.1]:46569 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uaapr-0006UB-KA for submit@debbugs.gnu.org; Sat, 12 Jul 2025 10:04:15 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:38694) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uaapp-0006Tf-0h for 78943@debbugs.gnu.org; Sat, 12 Jul 2025 10:04:13 -0400 Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 614894409E4; Sat, 12 Jul 2025 10:04:06 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1752329045; bh=USxj8a7BXGIhmk8I1CqFecEVpGZLSBXNCoF7kOotdLw=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=gSlvoWSyEvozXnLnp/pWtOZEz8W9X0YHIdHidhIK0Q5v42oiA4OkC2FMBD/wzLXg/ P+54aad7O2voOQY3JoaByR4EUYTNV7i9t15JNdeZTR+lj/5Di7Mdk6GhVmzRPzdZGR NmIVW3qjokMvun/MV090qg7NpPV5MebT7tcOijYuyBgmamc3ZRf4dLiBU5pRcpeBrj XebHckokRe3yL5aZhJUe9Hu8hibw7f5rzIZIvkXCg33fi8Y7soKikzTfTDbndKUO5n ZwGYnf3KqynVlDVkN2WH8Tl/0BhxvGm9Bc1HS7WMm0tUwZ6ZBLyLJlh8znii3TgADX dPMM0SurM8Q1w== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 6C3F544097B; Sat, 12 Jul 2025 10:04:05 -0400 (EDT) Received: from pastel (unknown [104.247.225.139]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 38EE61206DB; Sat, 12 Jul 2025 10:04:05 -0400 (EDT) From: Stefan Monnier To: Eli Zaretskii Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: <86v7nyexrk.fsf@gnu.org> Message-ID: References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <86v7nyexrk.fsf@gnu.org> Date: Sat, 12 Jul 2025 10:04:04 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.275 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78943 Cc: pipcet@protonmail.com, Helmut Eller , 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) >> 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. Could we adjust this size dynamically, keeping a high-water-mark? This way GC allocation would happen only when the high-water mark is changed (if it goes beyond a preset threshold if we care about contrived situations with thousands of minor mode maps). > Adding Stefan to the discussion. Fixing the API of `current_minor_modes` sounds good to me. Stefan From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 12 11:54:10 2025 Received: (at 78943) by debbugs.gnu.org; 12 Jul 2025 15:54:10 +0000 Received: from localhost ([127.0.0.1]:46885 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uacYE-00055N-Hi for submit@debbugs.gnu.org; Sat, 12 Jul 2025 11:54:10 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:17356) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uacYC-000555-Jy for 78943@debbugs.gnu.org; Sat, 12 Jul 2025 11:54:09 -0400 Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 70C7F440A63; Sat, 12 Jul 2025 11:54:01 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1752335640; bh=0iD9qgNVe/Y1YxJZOCnNmz07rUOI0oox9fOQ2ZXjj3o=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=mZTJ48TjpLgsg7bXD+QUuBjSFbELAT8gOBayt5yd76uZf3/n/hvkWDPLh3Bvf2UNq v/s+zk5VT3Jhp6oMHnibRsdSFoR6s6exOyUJKwzB8G82+gIkm4ySwxWcM81eNKqi7A 2pcGORaDwrLQpHiRYjftolfq2rYJtTyy5l4Jfg7zCWyCEOwLqswmp7RVNhork8VPGF dCHo47L8mmfgGqaxDssBdrjjsiogdfvfTw55ZHJSsf8WVF3eK8As+W59NCBMO5St/2 OsfvelbwuVGQJuaBjl0u/0dGWo1HoOsM9iD33B86EO8bBu1SODo4fDYbm1jgDDjLil xwuIclsPHORbQ== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 73B0E4409E7; Sat, 12 Jul 2025 11:54:00 -0400 (EDT) Received: from pastel (unknown [104.247.225.139]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 3ED0512010F; Sat, 12 Jul 2025 11:54:00 -0400 (EDT) From: Stefan Monnier To: Eli Zaretskii Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: Message-ID: References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <86v7nyexrk.fsf@gnu.org> Date: Sat, 12 Jul 2025 11:53:59 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.273 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78943 Cc: pipcet@protonmail.com, Helmut Eller , 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Fixing the API of `current_minor_modes` sounds good to me. ^^^^^ maps - Stefan From debbugs-submit-bounces@debbugs.gnu.org Sun Jul 13 11:31:47 2025 Received: (at 78943) by debbugs.gnu.org; 13 Jul 2025 15:31:47 +0000 Received: from localhost ([127.0.0.1]:54448 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uayg6-0006VL-SS for submit@debbugs.gnu.org; Sun, 13 Jul 2025 11:31:47 -0400 Received: from mail-ed1-x52e.google.com ([2a00:1450:4864:20::52e]:52359) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1uayg4-0006V3-7T for 78943@debbugs.gnu.org; Sun, 13 Jul 2025 11:31:44 -0400 Received: by mail-ed1-x52e.google.com with SMTP id 4fb4d7f45d1cf-607434e1821so4945995a12.0 for <78943@debbugs.gnu.org>; Sun, 13 Jul 2025 08:31:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752420698; x=1753025498; darn=debbugs.gnu.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=Ho5wlcLlbK+FJQdI/LY+bfVUSI9Zu3EcQBO2DnRErjc=; b=Qol98GBKm0YIrGbfeHFXWsDT9Prm29xb9hur+rE68XoK0sgmJgv3O+0W9x8v6Razfq lwNYE4/37YA/rR4NpZOZFDXpdpEldOTdGEISFsTfo0yclejAYT08esqh/iivNeBvlh0+ nhwEieyoduihFNUFJLyMTM5yK7wnIFpAXU/ShBGlCDnPV1ZDMYoMCTGhLuJ37KCHMMWg nxK99fcx+Mhuu0XH3m8zOvCLh20Rcqj8QkMvkUUEA47Dk+d6NDuoKVIusTEltaz4vU6P cB31kLa4iiBRCidKRSyjdNbN3x7F+dFRxK6m9mH9Qc2e1MmBWS96mEe98ceqA9xfVRjv 3fUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752420698; x=1753025498; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Ho5wlcLlbK+FJQdI/LY+bfVUSI9Zu3EcQBO2DnRErjc=; b=b5rxUASrjI1chpQKVXgcl0NqLnqwEXoKkCNN3O+q0jYBWUBdt8ZJSk+kmfqbqV899G H/v1IT1OEWtsYr9uZAVEt40hcUbGXSAz1I4aPb8uBigvzK3vWpVVAfUOQr3ENKpuOwad tsiP3lBZLFZuyHx3ZfLzNW+EWJF+ddZNNBEoakm8k4MjhrOIxJxq+GoHLSXJqZikfFLP rSPSwjigHKnZYA04DdfYpVH2biGVHywAdSO9oRhtj3M269AOR5Wdr0KXaovfzWxZSs3D GJyQM9dO0SDW/BwHbiI5CUxiu4trX1nnf7JL/ZVIka2p4alfg7Wa/Jrtl/APx+CsdmNc o/dA== X-Forwarded-Encrypted: i=1; AJvYcCUtsOstOnLzlNFRgUFGTkmW1oScnIbrgRXQH2kos4Oi+kgDx3xPSwQAExrJTakJv8w2Am2D1g==@debbugs.gnu.org X-Gm-Message-State: AOJu0Yw3Qk+n3N8fsXST2bZ/pl4Hx041asaIBYNR9LgsDp7RKgTl7AZr oMhJUQiQeBbhLo/hdQejyGNStxXYMV9F0l9AF/5NpaR/7Yx/K0tj4o0J0TfLXA== X-Gm-Gg: ASbGncuOlwjukwbrZIK+kJCEuYkUOlf9/xI1QtwC+RNIuM6n1AwEXq8zf0omLWz4EkZ gKEQDYAs8wmxoZhqDeqkAzbTWXjaauFfmTL5MnhNF3qB40tFtG1elZTJ2Wxlhd3hrDRnpxMER9B Pu20DFd5V/LsT2xymr/hJWSvVP9jNy6zrEqNXDH+f0s0FGBJeU6Ivlsdd6CNtppsk5/3z2uSlzE y+PL9wsnyBR8uLCKu2OCW0bY9bBMl1oX9XW/EEWf6kCVwex7+2VHrc/qyZFfzdm2ULktgNkIh7C oXAC64Ogvlhlj4LET9/bkUrmcpNObk9G3yI0JSnXJIxs+oCrqYwI89++XfrmIIx8mDTi1cLAENO PT+OvO807tRlLxdWI/AY= X-Google-Smtp-Source: AGHT+IEBM3REPjDDfrlgyWuLMr6GwveojvTgusIgMBSc8v0vxQErheUhip7L+R2TGxRZuMNeRVYb5A== X-Received: by 2002:a05:6402:3549:b0:609:b5e0:598a with SMTP id 4fb4d7f45d1cf-611e84d73femr9438616a12.24.1752420697466; Sun, 13 Jul 2025 08:31:37 -0700 (PDT) Received: from caladan ([31.177.115.39]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-611c9524072sm4962892a12.20.2025.07.13.08.31.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 13 Jul 2025 08:31:36 -0700 (PDT) From: Helmut Eller To: Stefan Monnier Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <86v7nyexrk.fsf@gnu.org> Date: Sun, 13 Jul 2025 17:31:36 +0200 Message-ID: <87o6tof6p3.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: Eli Zaretskii , pipcet@protonmail.com, 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) On Sat, Jul 12 2025, Stefan Monnier wrote: >>> 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. > > Could we adjust this size dynamically, keeping a high-water-mark? > This way GC allocation would happen only when the high-water mark is > changed (if it goes beyond a preset threshold if we care about > contrived situations with thousands of minor mode maps). The only way I can think of is with alloca/SAVE_ALLOCA. GC allocation could be avoided entirely if we are willing to call current_minor_maps twice with when the number of minior mode maps changes: size_t size; Lisp_Object *buffer; static size_t predicted_size = 0; do { size = predicted_size; buffer = SAFE_ALLOCA (size * word_size); predicted_size = current_minor_modes (size, buffer) } while (size != predicted_size) But this is not exactly a nice API and it can't be packed up as a function because of alloca. >> Adding Stefan to the discussion. > > Fixing the API of `current_minor_modes` sounds good to me. There is a lot of duplicated code in the functions menu_bar_items, tool_bar_items, and tab_bar_items. The duplicated code builds an array of keymaps. I wonder if we couldn't replace this with Fcurrent_active_maps? It seems to do something very similar. From debbugs-submit-bounces@debbugs.gnu.org Sun Jul 13 18:34:01 2025 Received: (at 78943) by debbugs.gnu.org; 13 Jul 2025 22:34:01 +0000 Received: from localhost ([127.0.0.1]:56594 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ub5Gi-0005wa-Nl for submit@debbugs.gnu.org; Sun, 13 Jul 2025 18:34:01 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:65150) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1ub5Gg-0005w2-5r for 78943@debbugs.gnu.org; Sun, 13 Jul 2025 18:33:58 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 998AA1000BC; Sun, 13 Jul 2025 18:33:52 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1752446031; bh=jKoaLdC89e/mO9v/SNuDA609QJcWf3GX2c3kP4fHmjE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=SjDl0fsBE59w3b53xTpBKxhTLtzMmndf96fABmSC3U3e+n2sWXPh3DI1r9xKzsLth jQT/QlqgEChTzo+TXyLdc3QvW7r8yRnK+HwceVGMIBqzW2j1RoLomK65Lf+f4uBAao QiN45pr3PM/+QQlzHtUZ/pyHHFV/6yhHMSi95dumBnCXKc5UANUScYlI1jc+F1m9oj PI0oL/3P+FjZiorBb6VY6/sHJKDYRH3JZtfgJgBpZ6Ct4EcMA4CS7jw7PnPQDIVAKp ak/dQ0i66ep2Mmqvtclw1jXSj1tYQUy36eqFUeCdH/kf0rITvWKx4unZ3GZcDM5WdN wZDJaVqG0w6gQ== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id BF43C100029; Sun, 13 Jul 2025 18:33:51 -0400 (EDT) Received: from alfajor (unknown [104.247.225.139]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 835CA120865; Sun, 13 Jul 2025 18:33:51 -0400 (EDT) From: Stefan Monnier To: Helmut Eller Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: <87o6tof6p3.fsf@gmail.com> Message-ID: References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <86v7nyexrk.fsf@gnu.org> <87o6tof6p3.fsf@gmail.com> Date: Sun, 13 Jul 2025 18:33:53 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.276 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 78943 Cc: Eli Zaretskii , pipcet@protonmail.com, 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > But this is not exactly a nice API and it can't be packed up as > a function because of alloca. We have such things in the C code, but I'm not fond of them, indeed. >> Fixing the API of `current_minor_modes` sounds good to me. > There is a lot of duplicated code in the functions menu_bar_items, > tool_bar_items, and tab_bar_items. The duplicated code builds an array > of keymaps. I wonder if we couldn't replace this with > Fcurrent_active_maps? It seems to do something very similar. IIRC there are some subtle issues that get in the way, mostly about ordering, but last time I looked at it was more than 10 years ago, so it might be worth revisiting. Stefan From debbugs-submit-bounces@debbugs.gnu.org Tue Jul 15 13:12:21 2025 Received: (at 78943) by debbugs.gnu.org; 15 Jul 2025 17:12:21 +0000 Received: from localhost ([127.0.0.1]:43562 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ubjCV-000869-4y for submit@debbugs.gnu.org; Tue, 15 Jul 2025 13:12:21 -0400 Received: from mail-ej1-x62f.google.com ([2a00:1450:4864:20::62f]:47196) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1ubjCP-00085c-Fd for 78943@debbugs.gnu.org; Tue, 15 Jul 2025 13:12:16 -0400 Received: by mail-ej1-x62f.google.com with SMTP id a640c23a62f3a-ae0de0c03e9so985513966b.2 for <78943@debbugs.gnu.org>; Tue, 15 Jul 2025 10:12:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1752599527; x=1753204327; darn=debbugs.gnu.org; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=peU3Ndl6hIwN0TKgHyPSv0bfNFYj9+1h/fbAgPAjBFQ=; b=TG3GoYBqEY58YmtHJDVkH4U2rXSuqQFJgjilRjy0ld7qnt7Jhpb2hlLUh6Lw33kVyF XwT7eLAO90k884Ce34MKpLGpDEiTBzpW4PXKMJFNNJMbpW8GXH6na/z0xeDyg6XcWnXM EOyWxP3FQVOuqSxKX9OnE6RFynGemUBDUrTNO4pDzvyG61TeYkcWmXTzwaf+TTWyUX6b dSitV1tSrBftKVeeet6QsbXFvrJZ9fBLgPcIoyjsOHTDGBc1qCUc1EVM9crIqiRw8Gxu 06Xa+BYrMH+/Shd5JeZTzKeSPVYrCa5vqSstQJF7nnZhm2vA40yM5VigTtIAzBgd6KbL Nxig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752599527; x=1753204327; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=peU3Ndl6hIwN0TKgHyPSv0bfNFYj9+1h/fbAgPAjBFQ=; b=K/6pz40yBXhNJEEb5GZWss37HlycHCHGVQBNORNdPSz2Si3o+zd342LGx1+gKGk8do cTWj33Ur4uKKc7VdA8VX5/JadAhKD5Xj+ax8AQO0wLvdqUtNLceXPNWGUTopAPVT4I9P vzdm9I6pj/9SdSpjyBO+ZkAtm4SYfacP/gvQxFWVU0NnOZZE/Q4/CYu6nsTk6pQDO6j7 3ZRpHKZQFaFpHrxXZxWGWEHQxU9AaVso2eJ/1AMG3q2EOC7vmT7Ad8POQxYcyXcpGi4f sLZdeeO6wRUErrx5YqiQVx8otZcz7nCPZk+IB6kHW6qAfvlRrfukUfWS1kJixUChTN0c u1bw== X-Gm-Message-State: AOJu0YxuteOcmOSlmWtSyzBnoREkZx1nSDlCswujNxMnmd5GQPWCSEPv 3Lj+zP+ISibSaOBKVuEcEJMkighHktg2D7ZQ7lE1leC4RSx5NP3YO/9Xg5+tRQ== X-Gm-Gg: ASbGnctDTdWFSHrt4zGhlBmNcOL2GR4wpBQFLCjEDq8+DW1rOZemkLTSl/rmC45wekP QbLzs79bf1H8vf1AM2ixB82Ai7dA8JDNod16Vem5c9DruMEg4+iIhwYXTGhImEWUVdOudlVKqL3 xvhYEiVZ0p+Rpmb6y4rztJYHrJZ+p/g2UJuWiL3YHNYahw/vdDE18BTStycpLNVXcSy4z5tT9VT jdBvqnr1LLu6QMOi1rOXhC0lqgAIG8+xQUh3EMYOWd0N2ArDG9TUxHXTPk+Mmk75SAF0YYB3voU mK6RW2KrWVinLIdBSjXpLLkFJD5McFLmBoZVnAQJDhaYA/n6/3DOy+Y63uqHdscIZR2+dDCrrbI DB9sZv2KFWuVGHgH6ZII= X-Google-Smtp-Source: AGHT+IEw7MxtDkIErBNfiJopSSUciejYfuhdtkSlWAsHGP69PyOn4uCeDLkKDyqz/4P1UOSU365etQ== X-Received: by 2002:a17:907:7248:b0:ae0:a359:a95c with SMTP id a640c23a62f3a-ae9c9ac0fc0mr30595566b.34.1752599526368; Tue, 15 Jul 2025 10:12:06 -0700 (PDT) Received: from caladan ([31.177.115.39]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ae6e8265bbesm1016862366b.80.2025.07.15.10.12.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Jul 2025 10:12:05 -0700 (PDT) From: Helmut Eller To: Pip Cet Subject: Re: bug#78943: feature/igc [PATCH] Trace current minor maps exactly In-Reply-To: <87jz4dpvbh.fsf@protonmail.com> References: <87o6u2ixoj.fsf@gmail.com> <87a55dsaii.fsf@protonmail.com> <8734b2h1s8.fsf@gmail.com> <87jz4dpvbh.fsf@protonmail.com> Date: Tue, 15 Jul 2025 19:12:05 +0200 Message-ID: <87wm89e5ui.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 78943 Cc: 78943@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain On Sat, Jul 12 2025, Pip Cet wrote: > Thanks. It'd be nice to have a more general approach to growable > vectors, but in this case, I don't understand why we use array > structures at all: two simple linked lists should work just as well. It seems that arrays are used because in some places the active minor maps need to be processed in the reversed order from the order they are stored in minor-mode-map-alist. [...] > My idea still would be to grow a vector aggressively until it's large > enough to fit all elements, then "truncate" it in place in such a way > that the GC knows to shrink it when it gets copied to the next > generation. This could work with MPS but I'm not so sure about the classic GC. Here is a bit a different approach that doesn't use the cmm_vector: 1. adds some test cases for the following changes 2. uses Fcurrent_active_maps in keyboard.c and replaces the duplicated code with a common function 3. replaces the remaining uses of current_minor_maps with functions that create some temporary lists 4. This is an optional improvement for 2, if we are hellbent on avoiding consing & reversing the list returned from Fcurrent_active_maps. This patch avoids the list at the cost of some complexity and alloca. --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Add-some-minor-mode-keymap-related-tests.patch >From a277abba2f10c94af3d57ffae62570ec7f3fce0b Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Mon, 14 Jul 2025 20:41:33 +0200 Subject: [PATCH 1/4] Add some minor-mode keymap related tests * test/src/keymap-tests.el (keymap-tests--current-active-maps/global-map) (keymap-tests--current-active-maps/local-map) (keymap-tests--current-active-maps/minor-maps) (keymap-tests--current-active-maps/olm) (keymap-tests--current-minor-mode-maps) (keymap-tests--minor-mode-key-binding): New tests. --- test/src/keymap-tests.el | 122 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el index c605c3eb09d..2acf57f9b82 100644 --- a/test/src/keymap-tests.el +++ b/test/src/keymap-tests.el @@ -509,6 +509,128 @@ keymap-unset-test-remove-and-inheritance ;; From the parent this time/ (should (equal (keymap-lookup map "u") #'undo)))) + +;;;; current-active-maps + +(ert-deftest keymap-tests--current-active-maps/global-map () + (with-temp-buffer + (should (equal (current-active-maps) (list global-map))))) + +(ert-deftest keymap-tests--current-active-maps/local-map () + (with-temp-buffer + (let ((local (make-sparse-keymap "local"))) + (use-local-map local) + (should (equal (current-active-maps) (list local global-map))) + (let ((m1 (make-sparse-keymap "m1")) + (m2 (make-sparse-keymap "m2"))) + (insert "abcd") + (add-text-properties 2 3 (list 'keymap m1 'local-map m2)) + (should (equal (current-active-maps nil 1) (list local global-map))) + (should (equal (current-active-maps nil 2) (list m1 m2 global-map))) + ;; 'keymap and 'local-map are "rear-sticky" + (should (equal (current-active-maps nil 3) (list m1 m2 global-map))) + (should (equal (current-active-maps nil 4) + (list local global-map))))))) + +(defvar keymap-tests-mm-2-x-map (make-sparse-keymap "mm-2 x")) +(defvar keymap-tests-mm-4-x-map (make-sparse-keymap "mm-4 x")) + +(defvar-keymap keymap-tests-minor-mode-2-map "x" keymap-tests-mm-2-x-map) +(defvar-keymap keymap-tests-minor-mode-3-map "x" 'keymap-tests--command-3) +(defvar-keymap keymap-tests-minor-mode-4-map "x" keymap-tests-mm-4-x-map) + +(define-minor-mode keymap-tests-minor-mode-2 "mm 2.") +(define-minor-mode keymap-tests-minor-mode-3 "mm 3.") +(define-minor-mode keymap-tests-minor-mode-4 "mm 4.") + +(ert-deftest keymap-tests--current-active-maps/minor-maps () + (with-temp-buffer + (keymap-tests-major-mode) + (keymap-tests-minor-mode) + (keymap-tests-minor-mode-3) + (should (equal (current-active-maps) + (list keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-map + keymap-tests-major-mode-map + global-map)))) + ;; the order of the minor maps is determined by minor-mode-map-alist + (with-temp-buffer + (keymap-tests-major-mode) + (keymap-tests-minor-mode-3) + (keymap-tests-minor-mode) + (should (equal (current-active-maps) + (list keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-map + keymap-tests-major-mode-map + global-map))))) + +(ert-deftest keymap-tests--current-active-maps/olm () + (with-temp-buffer + (keymap-tests-major-mode) + (keymap-tests-minor-mode) + (keymap-tests-minor-mode-3) + (let ((overriding-local-map (make-sparse-keymap "olp"))) + (should (equal (current-active-maps t) + (list overriding-local-map + global-map))) + (should (equal (current-active-maps nil) + (list keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-map + keymap-tests-major-mode-map + global-map)))) + (let ((overriding-terminal-local-map (make-sparse-keymap "otlm"))) + (should (equal (current-active-maps t) + (list overriding-terminal-local-map + keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-map + keymap-tests-major-mode-map + global-map)))) + (let ((overriding-terminal-local-map (make-sparse-keymap "otlm")) + (overriding-local-map (make-sparse-keymap "olm")) + (m1 (make-sparse-keymap "m1")) + (m2 (make-sparse-keymap "m2"))) + (insert "abcd") + (add-text-properties 2 3 (list 'keymap m1 'local-map m2)) + (should (equal (current-active-maps t 2) + (list overriding-terminal-local-map + m1 + keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-map + m2 + global-map)))))) + + +;;;; current-minor-mode-maps + +(ert-deftest keymap-tests--current-minor-mode-maps () + (with-temp-buffer + (keymap-tests-minor-mode) + (keymap-tests-minor-mode-3) + (keymap-tests-minor-mode-2) + (keymap-tests-minor-mode-4) + (should (equal (current-minor-mode-maps) + (list keymap-tests-minor-mode-4-map + keymap-tests-minor-mode-3-map + keymap-tests-minor-mode-2-map + keymap-tests-minor-mode-map))))) + +(ert-deftest keymap-tests--minor-mode-key-binding () + (with-temp-buffer + (keymap-tests-minor-mode) + (keymap-tests-minor-mode-2) + (keymap-tests-minor-mode-3) + (should (equal (minor-mode-key-binding "x") + '((keymap-tests-minor-mode-3 . keymap-tests--command-3))))) + (with-temp-buffer + (keymap-tests-minor-mode) + (keymap-tests-minor-mode-2) + (keymap-tests-minor-mode-3) + (keymap-tests-minor-mode-4) + (should + (equal (minor-mode-key-binding "x") + `((keymap-tests-minor-mode-4 . ,keymap-tests-mm-4-x-map) + (keymap-tests-minor-mode-2 . ,keymap-tests-mm-2-x-map)))))) + (provide 'keymap-tests) ;;; keymap-tests.el ends here -- 2.39.5 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Refactor-menu_bar_items-tab_bar_items-and-tool_bar_i.patch >From 5bb20d21b29f56c56416ae7d748a497eb19dfc8c Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Mon, 14 Jul 2025 21:44:37 +0200 Subject: [PATCH 2/4] Refactor menu_bar_items, tab_bar_items, and tool_bar_items Move duplicated code to the new function process_prefix_maps that the others can call. Also use Fcurrent_active_maps instead of manually building an array of active keymaps. * src/keyboard.c (process_prefix_maps): New function. (menu_bar_items, tab_bar_items, tool_bar_items): Use it. (menu_bar_item): Reset menu_bar_one_keymap_changed_items here. --- src/keyboard.c | 233 +++++++++---------------------------------------- 1 file changed, 41 insertions(+), 192 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index b389d98feaa..4dd38f13bb7 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -8568,32 +8568,41 @@ menu_separator_name_p (const char *label) return 0; } +/* Look up in each active map the event PREFIX (e.g. Qmenu_bar) and if + the result is a keymap, then call F for each binding in that keymap. + + The active maps are processed in reverse order, i.e. global-map + first. */ +static void +process_prefix_maps (Lisp_Object prefix, map_keymap_function_t f) +{ + for (Lisp_Object maps = Fnreverse (Fcurrent_active_maps (Qnil, Qnil)); + !NILP (maps); maps = XCDR (maps)) + { + Lisp_Object map = XCAR (maps); + Lisp_Object def = get_keymap (access_keymap (map, prefix, 1, 0, 1), + 0, 1); + if (CONSP (def)) + map_keymap_canonical (def, f, Qnil, NULL); + } +} /* Return a vector of menu items for a menu bar, appropriate to the current buffer. Each item has three elements in the vector: KEY STRING MAPLIST. - OLD is an old vector we can optionally reuse, or nil. */ + OLD is an old vector we can optionally reuse, or nil. + + Note that menu-bar bindings in the local-map and keymap properties + may not work reliable, as they are only recognized when the menu-bar + (or mode-line) is updated, which does not normally happen after every + command. */ Lisp_Object menu_bar_items (Lisp_Object old) { - /* The number of keymaps we're scanning right now, and the number of - keymaps we have allocated space for. */ - ptrdiff_t nmaps; - - /* maps[0..nmaps-1] are the prefix definitions of KEYBUF[0..t-1] - in the current keymaps, or nil where it is not a prefix. */ - Lisp_Object *maps; - - Lisp_Object mapsbuf[3]; - Lisp_Object def; - - ptrdiff_t mapno; Lisp_Object oquit; - USE_SAFE_ALLOCA; - /* In order to build the menus, we need to call the keymap accessors. They all call maybe_quit. But this function is called during redisplay, during which a quit is fatal. So inhibit @@ -8609,64 +8618,7 @@ menu_bar_items (Lisp_Object old) menu_bar_items_vector = make_nil_vector (24); menu_bar_items_index = 0; - /* Build our list of keymaps. - If we recognize a function key and replace its escape sequence in - keybuf with its symbol, or if the sequence starts with a mouse - click and we need to switch buffers, we jump back here to rebuild - the initial keymaps from the current buffer. */ - { - Lisp_Object *tmaps; - - /* Should overriding-terminal-local-map and overriding-local-map apply? */ - if (!NILP (Voverriding_local_map_menu_flag) - && !NILP (Voverriding_local_map)) - { - /* Yes, use them (if non-nil) as well as the global map. */ - maps = mapsbuf; - nmaps = 0; - if (!NILP (KVAR (current_kboard, Voverriding_terminal_local_map))) - maps[nmaps++] = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (Voverriding_local_map)) - maps[nmaps++] = Voverriding_local_map; - } - else - { - /* No, so use major and minor mode keymaps and keymap property. - Note that menu-bar bindings in the local-map and keymap - properties may not work reliable, as they are only - recognized when the menu-bar (or mode-line) is updated, - which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); - SAFE_NALLOCA (maps, 1, nminor + 4); - nmaps = 0; - Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (tem) && !NILP (Voverriding_local_map_menu_flag)) - maps[nmaps++] = tem; - if (tem = get_local_map (PT, current_buffer, Qkeymap), !NILP (tem)) - maps[nmaps++] = tem; - if (nminor != 0) - { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); - nmaps += nminor; - } - maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); - } - maps[nmaps++] = current_global_map; - } - - /* Look up in each map the dummy prefix key `menu-bar'. */ - - for (mapno = nmaps - 1; mapno >= 0; mapno--) - if (!NILP (maps[mapno])) - { - def = get_keymap (access_keymap (maps[mapno], Qmenu_bar, 1, 0, 1), - 0, 1); - if (CONSP (def)) - { - menu_bar_one_keymap_changed_items = Qnil; - map_keymap_canonical (def, menu_bar_item, Qnil, NULL); - } - } + process_prefix_maps (Qmenu_bar, menu_bar_item); /* Move to the end those items that should be at the end. */ @@ -8712,7 +8664,6 @@ menu_bar_items (Lisp_Object old) } Vinhibit_quit = oquit; - SAFE_FREE (); return menu_bar_items_vector; } @@ -8728,6 +8679,8 @@ menu_bar_item (Lisp_Object key, Lisp_Object item, Lisp_Object dummy1, void *dumm bool parsed; Lisp_Object tem; + menu_bar_one_keymap_changed_items = Qnil; + if (EQ (item, Qundefined)) { /* If a map has an explicit `undefined' as definition, @@ -9143,17 +9096,17 @@ parse_menu_item (Lisp_Object item, int inmenubar) /* Return a vector of tab bar items for keymaps currently in effect. Reuse vector REUSE if non-nil. Return in *NITEMS the number of - tab bar items found. */ + tab bar items found. + + Note that tab-bar bindings in the local-map and keymap properties may + not work reliably, as they are only recognized when the tab-bar (or + mode-line) is updated, which does not normally happen after every + command. */ Lisp_Object tab_bar_items (Lisp_Object reuse, int *nitems) { - Lisp_Object *maps; - Lisp_Object mapsbuf[3]; - ptrdiff_t nmaps, i; Lisp_Object oquit; - Lisp_Object *tmaps; - USE_SAFE_ALLOCA; *nitems = 0; @@ -9169,62 +9122,10 @@ tab_bar_items (Lisp_Object reuse, int *nitems) /* Initialize tab_bar_items_vector and protect it from GC. */ init_tab_bar_items (reuse); - /* Build list of keymaps in maps. Set nmaps to the number of maps - to process. */ - - /* Should overriding-terminal-local-map and overriding-local-map apply? */ - if (!NILP (Voverriding_local_map_menu_flag) - && !NILP (Voverriding_local_map)) - { - /* Yes, use them (if non-nil) as well as the global map. */ - maps = mapsbuf; - nmaps = 0; - if (!NILP (KVAR (current_kboard, Voverriding_terminal_local_map))) - maps[nmaps++] = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (Voverriding_local_map)) - maps[nmaps++] = Voverriding_local_map; - } - else - { - /* No, so use major and minor mode keymaps and keymap property. - Note that tab-bar bindings in the local-map and keymap - properties may not work reliably, as they are only - recognized when the tab-bar (or mode-line) is updated, - which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); - SAFE_NALLOCA (maps, 1, nminor + 4); - nmaps = 0; - Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (tem) && !NILP (Voverriding_local_map_menu_flag)) - maps[nmaps++] = tem; - if (tem = get_local_map (PT, current_buffer, Qkeymap), !NILP (tem)) - maps[nmaps++] = tem; - if (nminor != 0) - { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); - nmaps += nminor; - } - maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); - } - - /* Add global keymap at the end. */ - maps[nmaps++] = current_global_map; - - /* Process maps in reverse order and look up in each map the prefix - key `tab-bar'. */ - for (i = nmaps - 1; i >= 0; --i) - if (!NILP (maps[i])) - { - Lisp_Object keymap; - - keymap = get_keymap (access_keymap (maps[i], Qtab_bar, 1, 0, 1), 0, 1); - if (CONSP (keymap)) - map_keymap (keymap, process_tab_bar_item, Qnil, NULL, 1); - } + process_prefix_maps (Qtab_bar, process_tab_bar_item); Vinhibit_quit = oquit; *nitems = ntab_bar_items / TAB_BAR_ITEM_NSLOTS; - SAFE_FREE (); return tab_bar_items_vector; } @@ -9527,17 +9428,17 @@ append_tab_bar_item (void) /* Return a vector of tool bar items for keymaps currently in effect. Reuse vector REUSE if non-nil. Return in *NITEMS the number of - tool bar items found. */ + tool bar items found. + + Note that tool-bar bindings in the local-map and keymap properties + may not work reliably, as they are only recognized when the tool-bar + (or mode-line) is updated, which does not normally happen after every + command. */ Lisp_Object tool_bar_items (Lisp_Object reuse, int *nitems) { - Lisp_Object *maps; - Lisp_Object mapsbuf[3]; - ptrdiff_t nmaps, i; Lisp_Object oquit; - Lisp_Object *tmaps; - USE_SAFE_ALLOCA; *nitems = 0; @@ -9553,62 +9454,10 @@ tool_bar_items (Lisp_Object reuse, int *nitems) /* Initialize tool_bar_items_vector and protect it from GC. */ init_tool_bar_items (reuse); - /* Build list of keymaps in maps. Set nmaps to the number of maps - to process. */ - - /* Should overriding-terminal-local-map and overriding-local-map apply? */ - if (!NILP (Voverriding_local_map_menu_flag) - && !NILP (Voverriding_local_map)) - { - /* Yes, use them (if non-nil) as well as the global map. */ - maps = mapsbuf; - nmaps = 0; - if (!NILP (KVAR (current_kboard, Voverriding_terminal_local_map))) - maps[nmaps++] = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (Voverriding_local_map)) - maps[nmaps++] = Voverriding_local_map; - } - else - { - /* No, so use major and minor mode keymaps and keymap property. - Note that tool-bar bindings in the local-map and keymap - properties may not work reliably, as they are only - recognized when the tool-bar (or mode-line) is updated, - which does not normally happen after every command. */ - ptrdiff_t nminor = current_minor_maps (NULL, &tmaps); - SAFE_NALLOCA (maps, 1, nminor + 4); - nmaps = 0; - Lisp_Object tem = KVAR (current_kboard, Voverriding_terminal_local_map); - if (!NILP (tem) && !NILP (Voverriding_local_map_menu_flag)) - maps[nmaps++] = tem; - if (tem = get_local_map (PT, current_buffer, Qkeymap), !NILP (tem)) - maps[nmaps++] = tem; - if (nminor != 0) - { - memcpy (maps + nmaps, tmaps, nminor * sizeof (maps[0])); - nmaps += nminor; - } - maps[nmaps++] = get_local_map (PT, current_buffer, Qlocal_map); - } - - /* Add global keymap at the end. */ - maps[nmaps++] = current_global_map; - - /* Process maps in reverse order and look up in each map the prefix - key `tool-bar'. */ - for (i = nmaps - 1; i >= 0; --i) - if (!NILP (maps[i])) - { - Lisp_Object keymap; - - keymap = get_keymap (access_keymap (maps[i], Qtool_bar, 1, 0, 1), 0, 1); - if (CONSP (keymap)) - map_keymap (keymap, process_tool_bar_item, Qnil, NULL, 1); - } + process_prefix_maps (Qtool_bar, process_tool_bar_item); Vinhibit_quit = oquit; *nitems = ntool_bar_items / TOOL_BAR_ITEM_NSLOTS; - SAFE_FREE (); return tool_bar_items_vector; } -- 2.39.5 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0003-Eliminate-the-global-arrays-cmm_modes-and-cmm_maps.patch >From dcc83202622c50fca47aebdf90980acae3e27939 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Tue, 15 Jul 2025 13:23:17 +0200 Subject: [PATCH 3/4] Eliminate the global arrays cmm_modes and cmm_maps Replace the function current_minor_maps with traverse_minor_maps which doesn't use these arrays. * src/keymap.h (current_minor_maps): Deleted. * src/keymap.c (current_minor_maps): Deleted. (traverse_minor_maps): Traversal code factored out from the old current_minor_maps. (current_minor_mode_map_alist): New function. For cases where consing up a temporary list is acceptable. (prepend_minor_mode_keymaps): New function. For use in Fcurrent_active_maps. (struct tconc, tconc_push, tconc_append, prepend_minor_mode_keymaps_2) (current_minor_mode_map_alist_2): New helpers. (Fcurrent_active_maps, Fcurrent_minor_mode_maps): Use prepend_minor_mode_keymaps instead of current_minor_modes. (Fminor_mode_key_binding, Fdescribe_buffer_bindings): Use current_minor_mode_map_alist instead current_minor_modes. --- src/keymap.c | 251 ++++++++++++++++++++++++--------------------------- src/keymap.h | 1 - 2 files changed, 119 insertions(+), 133 deletions(-) diff --git a/src/keymap.c b/src/keymap.c index 2c250578b00..e6ae0b5abf2 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -1504,34 +1504,17 @@ silly_event_symbol_error (Lisp_Object c) /* Global, local, and minor mode keymap stuff. */ -/* We can't put these variables inside current_minor_maps, since under - some systems, static gets macro-defined to be the empty string. - Ickypoo. */ -static Lisp_Object *cmm_modes = NULL, *cmm_maps = NULL; -static ptrdiff_t cmm_size = 0; - -/* Store a pointer to an array of the currently active minor modes in - *modeptr, a pointer to an array of the keymaps of the currently - active minor modes in *mapptr, and return the number of maps - *mapptr contains. - - This function always returns a pointer to the same buffer, and may - free or reallocate it, so if you want to keep it for a long time or - hand it out to lisp code, copy it. This procedure will be called - for every key sequence read, so the nice lispy approach (return a - new assoclist, list, what have you) for each invocation would - result in a lot of consing over time. - - If we used xrealloc/xmalloc and ran out of memory, they would throw - back to the command loop, which would try to read a key sequence, - which would call this function again, resulting in an infinite - loop. Instead, we'll use realloc/malloc and silently truncate the - list, let the key sequence be read, and hope some other piece of - code signals the error. */ -ptrdiff_t -current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) -{ - ptrdiff_t i = 0; +/* Traverse the currently active minor modes, calling F for each + (minor-mode, keymap) pair. Abort the traversal early, iff F returns + false. + + Minor modes are visited in the same order as they occur in + Vminor_mode_map_alist and Vminor_mode_overriding_map_alist. */ +static void +traverse_minor_maps (bool (*f) (Lisp_Object mode, Lisp_Object map, + void *closure), + void *closure) +{ Lisp_Object alist, assoc, var, val; Lisp_Object emulation_alists = Vemulation_mode_map_alists; Lisp_Object lists[2]; @@ -1552,17 +1535,16 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) else alist = lists[list_number]; - for ( ; CONSP (alist); alist = XCDR (alist)) + for (; CONSP (alist); alist = XCDR (alist)) if ((assoc = XCAR (alist), CONSP (assoc)) && (var = XCAR (assoc), SYMBOLP (var)) - && (val = find_symbol_value (var), !BASE_EQ (val, Qunbound)) + && (val = find_symbol_value (var), + !BASE_EQ (val, Qunbound)) && !NILP (val)) { - Lisp_Object temp; - - /* If a variable has an entry in Vminor_mode_overriding_map_alist, - and also an entry in Vminor_mode_map_alist, - ignore the latter. */ + /* If a variable has an entry in + Vminor_mode_overriding_map_alist, and also an entry in + Vminor_mode_map_alist, ignore the latter. */ if (list_number == 1) { val = assq_no_quit (var, lists[0]); @@ -1570,68 +1552,83 @@ current_minor_maps (Lisp_Object **modeptr, Lisp_Object **mapptr) continue; } - if (i >= cmm_size) - { - ptrdiff_t newsize, allocsize; - Lisp_Object *newmodes, *newmaps; - - /* Check for size calculation overflow. Other code - (e.g., read_key_sequence) adds 3 to the count - later, so subtract 3 from the limit here. */ - if (min (PTRDIFF_MAX, SIZE_MAX) / (2 * sizeof *newmodes) - 3 - < cmm_size) - break; - - newsize = cmm_size == 0 ? 30 : cmm_size * 2; - allocsize = newsize * sizeof *newmodes; - - /* Use malloc here. See the comment above this function. - Avoid realloc here; it causes spurious traps on GNU/Linux [KFS] */ - block_input (); - newmodes = malloc (allocsize); - if (newmodes) - { - if (cmm_modes) - { - memcpy (newmodes, cmm_modes, - cmm_size * sizeof cmm_modes[0]); - free (cmm_modes); - } - cmm_modes = newmodes; - } + /* Get the keymap definition--or nil if it is not + defined. */ + Lisp_Object map = Findirect_function (XCDR (assoc), Qt); + if (NILP (map)) + continue; + if (!f (var, map, closure)) + return; + } + } +} - newmaps = malloc (allocsize); - if (newmaps) - { - if (cmm_maps) - { - memcpy (newmaps, cmm_maps, - cmm_size * sizeof cmm_maps[0]); - free (cmm_maps); - } - cmm_maps = newmaps; - } - unblock_input (); +/* Used for "tail concatenation". */ +struct tconc +{ + Lisp_Object head, tail; /* head is nil, if the list is empty */ +}; - if (newmodes == NULL || newmaps == NULL) - break; - cmm_size = newsize; - } +/* Append an element at the end of T. */ +static void +tconc_push (struct tconc *t, Lisp_Object item) +{ + Lisp_Object tail = list1 (item); + if (NILP (t->head)) + t->head = tail; + else + XSETCDR (t->tail, tail); + t->tail = tail; +} - /* Get the keymap definition--or nil if it is not defined. */ - temp = Findirect_function (XCDR (assoc), Qt); - if (!NILP (temp)) - { - cmm_modes[i] = var; - cmm_maps [i] = temp; - i++; - } - } - } +/* Concatenate T and LIST and return the result as a list. */ +static Lisp_Object +tconc_append (struct tconc t, Lisp_Object list) +{ + if (NILP (t.head)) + return list; + XSETCDR (t.tail, list); + return t.head; +} + +static bool +prepend_minor_mode_keymaps_2 (Lisp_Object mode, Lisp_Object map, + void *closure) +{ + struct tconc *l = closure; + eassert (!NILP (map)); + tconc_push (l, map); + return true; +} - if (modeptr) *modeptr = cmm_modes; - if (mapptr) *mapptr = cmm_maps; - return i; +/* Append the list of currently active minor mode keymaps and the list + KEYMAPS. */ +static Lisp_Object +prepend_minor_mode_keymaps (Lisp_Object keymaps) +{ + struct tconc tconc = { Qnil, Qnil }; + traverse_minor_maps (prepend_minor_mode_keymaps_2, &tconc); + return tconc_append (tconc, keymaps); +} + +static bool +current_minor_mode_map_alist_2 (Lisp_Object mode, Lisp_Object map, + void *closure) +{ + struct tconc *t = closure; + eassert (!NILP (map)); + tconc_push (t, Fcons (mode, map)); + return true; +} + +/* Return an alist ((MODE . KEYMAP) ...) of the currently active minor + modes. */ +static Lisp_Object +current_minor_mode_map_alist (void) +{ + struct tconc t = { Qnil, Qnil }; + traverse_minor_maps (current_minor_mode_map_alist_2, &t); + return t.head; } /* Return the offset of POSITION, a click position, in the style of @@ -1704,8 +1701,6 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, if (NILP (XCDR (keymaps))) { - Lisp_Object *maps; - int nmaps; ptrdiff_t pt = click_position (position); /* This usually returns the buffer's local map, but that can be overridden by a `local-map' property. */ @@ -1775,11 +1770,7 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, keymaps = Fcons (local_map, keymaps); /* Now put all the minor mode keymaps on the list. */ - nmaps = current_minor_maps (0, &maps); - - for (int i = --nmaps; i >= 0; i--) - if (!NILP (maps[i])) - keymaps = Fcons (maps[i], keymaps); + keymaps = prepend_minor_mode_keymaps (keymaps); if (!NILP (keymap)) keymaps = Fcons (keymap, keymaps); @@ -1876,23 +1867,21 @@ DEFUN ("minor-mode-key-binding", Fminor_mode_key_binding, Sminor_mode_key_bindin bindings; see the description of `lookup-key' for more details about this. */) (Lisp_Object key, Lisp_Object accept_default) { - Lisp_Object *modes, *maps; - int nmaps = current_minor_maps (&modes, &maps); - Lisp_Object binding = Qnil; - - int j; - for (int i = j = 0; i < nmaps; i++) - if (!NILP (maps[i]) - && !NILP (binding = Flookup_key (maps[i], key, accept_default)) - && !FIXNUMP (binding)) - { - if (KEYMAPP (binding)) - maps[j++] = Fcons (modes[i], binding); - else if (j == 0) - return list1 (Fcons (modes[i], binding)); - } - - return Flist (j, maps); + Lisp_Object prefix_bindings = Qnil; + for (Lisp_Object alist = current_minor_mode_map_alist (); + !NILP (alist); alist = XCDR (alist)) + { + Lisp_Object pair = XCAR (alist); + Lisp_Object mode = XCAR (pair); + Lisp_Object map = XCDR (pair); + eassert (!NILP (map)); + Lisp_Object binding = Flookup_key (map, key, accept_default); + if (KEYMAPP (binding)) + prefix_bindings = Fcons (Fcons (mode, binding), prefix_bindings); + else if (NILP (prefix_bindings)) + return list1 (Fcons (mode, binding)); + } + return Fnreverse (prefix_bindings); } DEFUN ("use-global-map", Fuse_global_map, Suse_global_map, 1, 1, 0, @@ -1937,10 +1926,7 @@ DEFUN ("current-minor-mode-maps", Fcurrent_minor_mode_maps, Scurrent_minor_mode_ doc: /* Return a list of keymaps for the minor modes of the current buffer. */) (void) { - Lisp_Object *maps; - int nmaps = current_minor_maps (0, &maps); - - return Flist (nmaps, maps); + return prepend_minor_mode_keymaps (Qnil); } /* Help functions for describing and documenting keymaps. */ @@ -2929,13 +2915,11 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b else { /* Print the minor mode and major mode keymaps. */ - Lisp_Object *modes, *maps; /* Temporarily switch to `buffer', so that we can get that buffer's minor modes correctly. */ Fset_buffer (buffer); - - int nmaps = current_minor_maps (&modes, &maps); + Lisp_Object cmms = current_minor_mode_map_alist (); Fset_buffer (outbuf); start1 = get_local_map (BUF_PT (XBUFFER (buffer)), @@ -2950,25 +2934,28 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b } /* Print the minor mode maps. */ - for (int i = 0; i < nmaps; i++) + for (; !NILP (cmms); cmms = XCDR (cmms)) { + Lisp_Object pair = XCAR (cmms); + Lisp_Object mode = XCAR (pair); + Lisp_Object map = XCDR (pair); /* The title for a minor mode keymap is constructed at run time. We let `help--describe-map-tree' do the actual insertion because it takes care of other features when doing so. */ char *title, *p; - if (!SYMBOLP (modes[i])) + if (!SYMBOLP (mode)) emacs_abort (); USE_SAFE_ALLOCA; - p = title = SAFE_ALLOCA (42 + SBYTES (SYMBOL_NAME (modes[i]))); + p = title = SAFE_ALLOCA (42 + SBYTES (SYMBOL_NAME (mode))); *p++ = '\f'; *p++ = '\n'; *p++ = '`'; - memcpy (p, SDATA (SYMBOL_NAME (modes[i])), - SBYTES (SYMBOL_NAME (modes[i]))); - p += SBYTES (SYMBOL_NAME (modes[i])); + memcpy (p, SDATA (SYMBOL_NAME (mode)), + SBYTES (SYMBOL_NAME (mode))); + p += SBYTES (SYMBOL_NAME (mode)); *p++ = '\''; memcpy (p, " Minor Mode Bindings", strlen (" Minor Mode Bindings")); p += strlen (" Minor Mode Bindings"); @@ -2976,9 +2963,9 @@ DEFUN ("describe-buffer-bindings", Fdescribe_buffer_bindings, Sdescribe_buffer_b Lisp_Object msg = build_unibyte_string (title); calln (Qhelp__describe_map_tree, - maps[i], Qt, shadow, prefix, + map, Qt, shadow, prefix, msg, nomenu, Qnil, Qnil, Qnil, buffer); - shadow = Fcons (maps[i], shadow); + shadow = Fcons (map, shadow); SAFE_FREE (); } diff --git a/src/keymap.h b/src/keymap.h index c9b47ffccd6..b93707b6368 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -36,7 +36,6 @@ #define KEYMAPP(m) (!NILP (get_keymap (m, false, false))) extern char *push_key_description (EMACS_INT, char *); extern Lisp_Object access_keymap (Lisp_Object, Lisp_Object, bool, bool, bool); extern Lisp_Object get_keymap (Lisp_Object, bool, bool); -extern ptrdiff_t current_minor_maps (Lisp_Object **, Lisp_Object **); extern void initial_define_lispy_key (Lisp_Object, const char *, const char *); extern void syms_of_keymap (void); -- 2.39.5 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0004-In-menu_bar_items-avoid-consing-reversing-the-list-o.patch >From 769275ebd14d0db0ecd5674e8f46ecac603c9286 Mon Sep 17 00:00:00 2001 From: Helmut Eller Date: Tue, 15 Jul 2025 17:33:06 +0200 Subject: [PATCH 4/4] In menu_bar_items, avoid consing & reversing the list of active maps * src/keymap.h (traverse_active_maps): New. * src/keymap.c (traverse_active_maps): Factored out from Fcurrent_active_maps. (Fcurrent_active_maps): Use it. (traverse_active_minor_maps, traverse_active_minor_maps_2) (struct traverse_active_minor_maps_env, traverse_active_maps_2) (struct traverse_active_maps_env, Fcurrent_active_maps_2): New helpers. * src/keyboard.c (process_prefix_maps): Use traverse_active_maps instead of Fcurrent_active_maps. (process_prefix_maps_2, struct process_prefix_maps_env): New helpers. --- src/keyboard.c | 27 ++++--- src/keymap.c | 194 ++++++++++++++++++++++++++++++++++++++----------- src/keymap.h | 4 + 3 files changed, 173 insertions(+), 52 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index 4dd38f13bb7..968720215ee 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -8568,6 +8568,22 @@ menu_separator_name_p (const char *label) return 0; } +struct process_prefix_maps_env { + map_keymap_function_t f; + Lisp_Object prefix; +}; + +static bool +process_prefix_maps_2 (Lisp_Object map, void *closure) +{ + struct process_prefix_maps_env *env = closure; + Lisp_Object def + = get_keymap (access_keymap (map, env->prefix, 1, 0, 1), 0, 1); + if (CONSP (def)) + map_keymap_canonical (def, env->f, Qnil, NULL); + return true; +} + /* Look up in each active map the event PREFIX (e.g. Qmenu_bar) and if the result is a keymap, then call F for each binding in that keymap. @@ -8576,15 +8592,8 @@ menu_separator_name_p (const char *label) static void process_prefix_maps (Lisp_Object prefix, map_keymap_function_t f) { - for (Lisp_Object maps = Fnreverse (Fcurrent_active_maps (Qnil, Qnil)); - !NILP (maps); maps = XCDR (maps)) - { - Lisp_Object map = XCAR (maps); - Lisp_Object def = get_keymap (access_keymap (map, prefix, 1, 0, 1), - 0, 1); - if (CONSP (def)) - map_keymap_canonical (def, f, Qnil, NULL); - } + struct process_prefix_maps_env env = { .f = f, .prefix = prefix }; + traverse_active_maps (false, Qnil, process_prefix_maps_2, &env); } /* Return a vector of menu items for a menu bar, appropriate diff --git a/src/keymap.c b/src/keymap.c index e6ae0b5abf2..e1ba1d309ef 100644 --- a/src/keymap.c +++ b/src/keymap.c @@ -1644,18 +1644,90 @@ click_position (Lisp_Object position) return pos; } -DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, - 0, 2, 0, - doc: /* Return a list of the currently active keymaps. -OLP if non-nil indicates that we should obey `overriding-local-map' and -`overriding-terminal-local-map'. POSITION can specify a click position -like in the respective argument of `key-binding' or a live window which -means to return the active maps for that window's buffer. */) - (Lisp_Object olp, Lisp_Object position) +struct traverse_active_minor_maps_env +{ + size_t capacity, len; + Lisp_Object *maps; +}; + +static bool +traverse_active_minor_maps_2 (Lisp_Object mode, Lisp_Object map, + void *closure) +{ + struct traverse_active_minor_maps_env *env = closure; + if (env->len < env->capacity) + env->maps[env->len] = map; + env->len++; + return true; +} + +/* This is similar to traverse_minor_maps but here the keymaps are + visited in the reversed order. */ +static void +traverse_active_minor_maps (bool (*f) (Lisp_Object map, + void *closure), + void *closure) +{ + struct traverse_active_minor_maps_env env; + USE_SAFE_ALLOCA; + do + { + /* We expect that the number of minor modes changes rarely and + hence last_len will be a good guess for the needed size. If + the guessed capacity is too small, then we need a second + round through the loop. */ + static size_t last_len = 0; + env.len = 0; + env.capacity = max (last_len, 8); + SAFE_ALLOCA_LISP (env.maps, env.capacity); + traverse_minor_maps (traverse_active_minor_maps_2, &env); + last_len = env.len; + } + while (env.capacity < env.len); + + for (size_t i = env.len; i != 0;) + { + i--; + Lisp_Object map = env.maps[i]; + if (!f (map, closure)) + break; + } + SAFE_FREE (); +} + +struct traverse_active_maps_env +{ + bool (*f) (Lisp_Object map, void *closure); + void *closure; + bool abort; +}; + +static bool +traverse_active_maps_2 (Lisp_Object map, void *closure) +{ + eassert (!NILP (map)); + struct traverse_active_maps_env *env = closure; + if (!env->f (map, env->closure)) + { + env->abort = true; + return false; + } + return true; +} + +/* Call F for each of the currently active keymaps. + + The keymaps are visited in reverse order of the list returned by + Fcurrent_active_maps, i.e. global-map comes first. */ +void +traverse_active_maps (bool olp, Lisp_Object position, + bool (*f) (Lisp_Object map, void *closure), + void *closure) { specpdl_ref count = SPECPDL_INDEX (); - Lisp_Object keymaps = list1 (current_global_map); + if (!f (current_global_map, closure)) + goto unwind; /* If a mouse click position is given, our variables are based on the buffer clicked on, not the current buffer. So we may have to @@ -1665,8 +1737,7 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, { Lisp_Object window = POSN_WINDOW (position); - if (WINDOWP (window) - && BUFFERP (XWINDOW (window)->contents) + if (WINDOWP (window) && BUFFERP (XWINDOW (window)->contents) && XBUFFER (XWINDOW (window)->contents) != current_buffer) { /* Arrange to go back to the original buffer once we're done @@ -1687,27 +1758,33 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, { /* See comment above. */ record_unwind_current_buffer (); - set_buffer_internal (XBUFFER (XWINDOW (position)->contents)); + set_buffer_internal ( + XBUFFER (XWINDOW (position)->contents)); } } - if (!NILP (olp) + if (olp /* The doc said that overriding-terminal-local-map should override overriding-local-map. The code used them both, but it seems clearer to use just one. rms, jan 2005. */ && NILP (KVAR (current_kboard, Voverriding_terminal_local_map)) && !NILP (Voverriding_local_map)) - keymaps = Fcons (Voverriding_local_map, keymaps); - - if (NILP (XCDR (keymaps))) + { + if (!f (Voverriding_local_map, closure)) + goto unwind; + } + else { ptrdiff_t pt = click_position (position); /* This usually returns the buffer's local map, but that can be overridden by a `local-map' property. */ - Lisp_Object local_map = get_local_map (pt, current_buffer, Qlocal_map); + Lisp_Object local_map + = get_local_map (pt, current_buffer, Qlocal_map); /* This returns nil unless there is a `keymap' property. */ - Lisp_Object keymap = get_local_map (pt, current_buffer, Qkeymap); - Lisp_Object otlp = KVAR (current_kboard, Voverriding_terminal_local_map); + Lisp_Object keymap + = get_local_map (pt, current_buffer, Qkeymap); + Lisp_Object otlp + = KVAR (current_kboard, Voverriding_terminal_local_map); if (CONSP (position)) { @@ -1719,11 +1796,12 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, if (POSN_INBUFFER_P (position)) { Lisp_Object pos = POSN_BUFFER_POSN (position); - if (FIXNUMP (pos) - && XFIXNUM (pos) >= BEG && XFIXNUM (pos) <= Z) + if (FIXNUMP (pos) && XFIXNUM (pos) >= BEG + && XFIXNUM (pos) <= Z) { - local_map = get_local_map (XFIXNUM (pos), - current_buffer, Qlocal_map); + local_map + = get_local_map (XFIXNUM (pos), current_buffer, + Qlocal_map); keymap = get_local_map (XFIXNUM (pos), current_buffer, Qkeymap); @@ -1740,46 +1818,76 @@ DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, { Lisp_Object pos = XCDR (string); string = XCAR (string); - if (FIXNUMP (pos) - && XFIXNUM (pos) >= 0 + if (FIXNUMP (pos) && XFIXNUM (pos) >= 0 && XFIXNUM (pos) < SCHARS (string)) { - Lisp_Object map = Fget_text_property (pos, Qlocal_map, - string); + Lisp_Object map + = Fget_text_property (pos, Qlocal_map, string); Lisp_Object pos_area = POSN_POSN (position); /* For clicks on mode line or header line, override - the maps we found at POSITION unconditionally, even - if the corresponding properties of the mode- or - header-line string are nil, because propertries at - point are not relevant in that case. */ - if (!NILP (map) - || EQ (pos_area, Qmode_line) + the maps we found at POSITION unconditionally, + even if the corresponding properties of the mode- + or header-line string are nil, because + propertries at point are not relevant in that + case. */ + if (!NILP (map) || EQ (pos_area, Qmode_line) || EQ (pos_area, Qheader_line)) local_map = map; map = Fget_text_property (pos, Qkeymap, string); - if (!NILP (map) - || EQ (pos_area, Qmode_line) + if (!NILP (map) || EQ (pos_area, Qmode_line) || EQ (pos_area, Qheader_line)) keymap = map; } } - } if (!NILP (local_map)) - keymaps = Fcons (local_map, keymaps); + if (!f (local_map, closure)) + goto unwind; - /* Now put all the minor mode keymaps on the list. */ - keymaps = prepend_minor_mode_keymaps (keymaps); + /* Now visit all the minor mode keymaps. */ + { + struct traverse_active_maps_env env + = { .f = f, .closure = closure, .abort = false }; + traverse_active_minor_maps (traverse_active_maps_2, &env); + if (env.abort) + goto unwind; + } if (!NILP (keymap)) - keymaps = Fcons (keymap, keymaps); + if (!f (keymap, closure)) + goto unwind; - if (!NILP (olp) && !NILP (otlp)) - keymaps = Fcons (otlp, keymaps); + if (olp && !NILP (otlp)) + if (!f (otlp, closure)) + goto unwind; } - return unbind_to (count, keymaps); + unwind: + unbind_to (count, Qnil); +} + +static bool +Fcurrent_active_maps_2 (Lisp_Object map, void *closure) +{ + Lisp_Object *keymaps = closure; + *keymaps = Fcons (map, *keymaps); + return true; +} + +DEFUN ("current-active-maps", Fcurrent_active_maps, Scurrent_active_maps, + 0, 2, 0, + doc: /* Return a list of the currently active keymaps. +OLP if non-nil indicates that we should obey `overriding-local-map' and +`overriding-terminal-local-map'. POSITION can specify a click position +like in the respective argument of `key-binding' or a live window which +means to return the active maps for that window's buffer. */) + (Lisp_Object olp, Lisp_Object position) +{ + Lisp_Object keymaps = Qnil; + traverse_active_maps (!NILP (olp), position, + Fcurrent_active_maps_2, &keymaps); + return keymaps; } /* GC is possible in this function if it autoloads a keymap. */ diff --git a/src/keymap.h b/src/keymap.h index b93707b6368..56494343559 100644 --- a/src/keymap.h +++ b/src/keymap.h @@ -46,5 +46,9 @@ #define KEYMAPP(m) (!NILP (get_keymap (m, false, false))) extern void map_keymap_canonical (Lisp_Object map, map_keymap_function_t fun, Lisp_Object args, void *data); +extern void traverse_active_maps (bool olp, Lisp_Object position, + bool (*f) (Lisp_Object map, + void *closure), + void *closure); #endif -- 2.39.5 --=-=-=--