From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#73725: Master: Wrong position for byte compiler warning message. Date: Wed, 23 Oct 2024 17:44:22 +0000 Message-ID: References: <8A05D4D2-C77B-4089-B82C-4DB3E88F1276@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="11706"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 73746@debbugs.gnu.org, acm@muc.de, Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= , 73725@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Oct 23 19:46:02 2024 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 1t3fQn-0002p0-CR for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 23 Oct 2024 19:46:01 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t3fQW-0006dH-FU; Wed, 23 Oct 2024 13:45:44 -0400 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 1t3fQU-0006cu-Eq for bug-gnu-emacs@gnu.org; Wed, 23 Oct 2024 13:45:42 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1t3fQU-0001dL-68 for bug-gnu-emacs@gnu.org; Wed, 23 Oct 2024 13:45:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=From:In-Reply-To:MIME-Version:References:Date:To:Subject; bh=A7Ob5poa5XUjN10N1XoiEO/lcsWWQbBznqWqmmA4mWo=; b=UERRMc9ny2Nzg4coMSUGYEV3uGpQIQK6trOMfjmGzitXn2EURmPN5V9TM9tdXSiso06bqbxt+IOCsGQxMtFAglV2JKE50DbTX9ssJSzSjtl9yv8a52g+5+BLeoQ6TMDXOgI8YeF1MQFLBpGYk7I7fvhDWAPYLU+LiN6jax+LUiG0Amh96ralSyHcc21xbNOfIPrCjlwDL7etWUKXsFtl5bTFZa9ISzJ7r9zzZmZZzqg6RyN0+u5YOTeoNXG0/PmP1AjVSZcDXko/5eHtGzzl003yp879S5Df56e9E2t0Bqd69NRuEF3WA3oztGYJfL9mz+mip4RghJfn/5dzAi39ew==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t3fQo-0007fU-1T for bug-gnu-emacs@gnu.org; Wed, 23 Oct 2024 13:46:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 23 Oct 2024 17:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73725 X-GNU-PR-Package: emacs Original-Received: via spool by 73725-submit@debbugs.gnu.org id=B73725.172970550329033 (code B ref 73725); Wed, 23 Oct 2024 17:46:02 +0000 Original-Received: (at 73725) by debbugs.gnu.org; 23 Oct 2024 17:45:03 +0000 Original-Received: from localhost ([127.0.0.1]:60690 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t3fPq-0007YC-TG for submit@debbugs.gnu.org; Wed, 23 Oct 2024 13:45:03 -0400 Original-Received: from mail.muc.de ([193.149.48.3]:20723) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t3fPn-0007XR-Tt for 73725@debbugs.gnu.org; Wed, 23 Oct 2024 13:45:01 -0400 Original-Received: (qmail 71261 invoked by uid 3782); 23 Oct 2024 19:44:22 +0200 Original-Received: from muc.de (pd953aa34.dip0.t-ipconnect.de [217.83.170.52]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Wed, 23 Oct 2024 19:44:22 +0200 Original-Received: (qmail 700 invoked by uid 1000); 23 Oct 2024 17:44:22 -0000 Content-Disposition: inline In-Reply-To: X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de 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:294150 Archived-At: Hello, Stefan. On Wed, Oct 23, 2024 at 09:27:12 -0400, Stefan Monnier wrote: > LGTM. Further nitpicks below, > Stefan > > diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el > > index d8dbfa62bf9..5cfc3528492 100644 > > --- a/lisp/emacs-lisp/byte-opt.el > > +++ b/lisp/emacs-lisp/byte-opt.el > > @@ -509,8 +509,12 @@ byte-optimize-one-form > > (defun byte-optimize-form (form &optional for-effect) > > (while > > (progn > > - ;; First, optimize all sub-forms of this one. > > - (setq form (byte-optimize-form-code-walker form for-effect)) > > + ;; First, optimize all sub-forms of this one. Note that > > + ;; `byte-optimize-form-code-walker' sometimes changes the car of > > + ;; `form', hence the `macroexp-preserve-posification'. > > + (setq form > > + (macroexp-preserve-posification > > + form (byte-optimize-form-code-walker form for-effect))) > We know it can change the `car`, that's not the question I think the > comment should answer. The comment should instead explain why > `byte-optimize-form-code-walker` doesn't use > `macroexp-preserve-posification` in those few places where it can change > the `car`s. > IIUC the answer is something like "because it was more work". Well, we could argue for some time as to whether 5 items from 20 is "few" or not. But I've amended the comment to say the invocation to m-p-posification is here for "source code economy". As I think I've already said, the slowdown on a make bootstrap is around 0.5%, barely measureable without using perf. > > @@ -524,7 +603,9 @@ macroexpand-all > > definitions to shadow the loaded ones for use in file byte-compilation." > > (let ((macroexpand-all-environment environment) > > (macroexp--dynvars macroexp--dynvars)) > > - (macroexp--expand-all form))) > > + (macroexp-preserve-posification > > + form > > + (macroexp--expand-all form)))) > I missed this one earlier: why do we need this one? Very good point. I vaguely remember going through this macro--expand-all when I was introducing the SWPs in the first place. However, there's one point in the function where the SWP isn't necessarily preserved, and that's near the end where compiler macros get handled. Here, we have less control over what the handler does. So I put an invocation of macroexp-preserve-posification into macroexp--compiler-macro to fix this, and took the one out of macroexpand-all as not needed, as you suggested. > Doesn't `macroexp--expand-all` already take care of > preserving positions? > [ You don't need to answer here: better answer in the code 🙂 ] I've done both! But I'm now confident enough about the patch not to send it to you for a third time. Thanks for all the feedback. > Stefan -- Alan Mackenzie (Nuremberg, Germany).