From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#67005: 30.0.50; improve nadivce/comp/trampoline handling Date: Mon, 20 Nov 2023 22:35:55 -0500 Message-ID: References: <874jhv9921.fsf@sappc2.fritz.box> <875y24zrt1.fsf@sappc2.fritz.box> <87ttpmwuxi.fsf@sappc2.fritz.box> <877cmct4a1.fsf@sappc2.fritz.box> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="29467"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: andrea corallo , 67005@debbugs.gnu.org To: Jens Schmidt Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Nov 21 04:37:22 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1r5HZh-0007Th-GB for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 21 Nov 2023 04:37:21 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1r5HZO-0006ec-HG; Mon, 20 Nov 2023 22:37:02 -0500 Original-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 1r5HZM-0006eD-Dp for bug-gnu-emacs@gnu.org; Mon, 20 Nov 2023 22:37:00 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1r5HZM-0004o8-5R for bug-gnu-emacs@gnu.org; Mon, 20 Nov 2023 22:37:00 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1r5HZO-0000Z9-4u for bug-gnu-emacs@gnu.org; Mon, 20 Nov 2023 22:37:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 21 Nov 2023 03:37:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67005 X-GNU-PR-Package: emacs Original-Received: via spool by 67005-submit@debbugs.gnu.org id=B67005.17005377902130 (code B ref 67005); Tue, 21 Nov 2023 03:37:02 +0000 Original-Received: (at 67005) by debbugs.gnu.org; 21 Nov 2023 03:36:30 +0000 Original-Received: from localhost ([127.0.0.1]:54897 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5HYn-0000YE-SE for submit@debbugs.gnu.org; Mon, 20 Nov 2023 22:36:29 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:46447) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r5HYY-0000Xn-Dg for 67005@debbugs.gnu.org; Mon, 20 Nov 2023 22:36:24 -0500 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 0C205100068; Mon, 20 Nov 2023 22:36:02 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1700537761; bh=aUzJT/2Il91WkSkKQ0yyAo4D/swo4ZMB9ZHJ9X2rAUE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=o8A2P187L6QBBOcT3b+m4xRLckQEqICumw+6G3rTT7KfTW8HGlmFeX5wRvj5iKYnA fmJsXFZLmm5SsjCfFmOtWjbgyI/VgYtcnUExnnMFDB+iuqvBWqVFwJjK+WkIob5h5x vlZ1ACbFlBL66wUDzFg6sQ8D8IKHuCRFa9pfaTBdQygr8I4MBJZo5M9UU+2lbnSMz4 HrV1zent3QwuluMdOJU3rmbB3C5mQAAjLaU0JeFtp2u1A9aqd5pFvFeYOZRBK9CZkn P3++/YMceIDL3JP/L/nUyh7RtBDPZzKzWsEX4exv/B6LjrEuTOi6wzgXbAL93ZoG6+ AI42gxK82ULiA== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 387C4100033; Mon, 20 Nov 2023 22:36:01 -0500 (EST) Original-Received: from pastel (unknown [45.72.227.120]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 0AF30120472; Mon, 20 Nov 2023 22:36:01 -0500 (EST) In-Reply-To: <877cmct4a1.fsf@sappc2.fritz.box> (Jens Schmidt's message of "Tue, 21 Nov 2023 00:04:54 +0100") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:274697 Archived-At: > (defcustom native-comp-never-optimize-functions > - '(eval > - ;; The following two are mandatory for Emacs to be working > - ;; correctly (see comment in `advice--add-function'). DO NOT > - ;; REMOVE. > - macroexpand rename-buffer) > + ;; Do not include functions `macroexpand' or `rename-buffer' as > + ;; default values here. Despite the previous "DO NOT REMOVE" > + ;; warnings these are no longer needed. See also the comment on > + ;; `advice--add-function' and bug#67005. FIXME: But do include > + ;; `eval' as temporary (?) remedy for bug#67141. > + '(eval) > "Primitive functions to exclude from trampoline optimization. I'm not sure I like the idea of keeping the whole history in comments. I suggest you try and trim it down to the part that seems likely to reoccur, like "We used to list those functions that were advised during preload but we now prefer to disallow them in `advice-add`". > In `addvice--add-function' I wanted to at least preserve the comment > from my initial patch (see attachment to > https://yhetil.org/emacs-bugs/874jhv9921.fsf@sappc2.fritz.box/). I > think it might help historical research if for that removal there is > something that a "git blame" could be hooked onto. `C-x v h` is your friend. It was weird in the fist place to put the trampoline stuff there (e.g. it's specific to functions stored in symbols so it would have been more logical to put it into `advice-add` instead), so it doesn't seem very likely that this will re-occur. > <<>>" > + ;; Actively disallow function advices (here) and advices in general > + ;; on primitives (in `comp--install-trampoline') during bootstrap > + ;; for the following reasons: > + ;; - Advices in Emacs' core are generally considered bad style. > + ;; - `Snarf-documentation' looses docstrings of advised dumped > + ;; functions (bug#66032#20). > + ;; - Native compilation does not generate trampolines for advised > + ;; primitives while loadup.el executes. I don't think this last point is true/relevant, is it? IIUC it would use a "normal funcall", which doesn't need a trampoline. > ;; TODO: > ;; - record the advice location, to display in describe-function. > - ;; - change all defadvice in lisp/**/*.el. > - ;; - obsolete advice.el. > + (when purify-flag > + (error "Invalid pre-dump advice on %s" symbol)) > (let* ((f (symbol-function symbol)) > > What do you think? Beside the comments about the comments, it's a +1 from me. Stefan