GNU bug report logs - #73725
Master: Wrong position for byte compiler warning message.

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 73746 <at> debbugs.gnu.org, Mattias Engdegård <mattias.engdegard <at> gmail.com>, 73725 <at> debbugs.gnu.org
Subject: bug#73725: Master: Wrong position for byte compiler warning message.
Date: Wed, 23 Oct 2024 09:27:12 -0400

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".

> @@ -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?
Doesn't `macroexp--expand-all` already take care of
preserving positions?
[ You don't need to answer here: better answer in the code 🙂  ]


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.