unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Alan Mackenzie <acm@muc.de>
Cc: "Mattias Engdegård" <mattias.engdegard@gmail.com>, 73725@debbugs.gnu.org
Subject: bug#73725: Master: Wrong position for byte compiler warning message.
Date: Fri, 11 Oct 2024 19:45:18 -0400	[thread overview]
Message-ID: <jwvttdii56q.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <Zweq-fxF2cNXL2nA@MAC.fritz.box> (Alan Mackenzie's message of "Thu, 10 Oct 2024 10:22:49 +0000")

> (i) Create the following file, bad-error-position2.el:
>
> #########################################################################
> ;; -*- lexical-binding:t -*-
> (eval-and-compile
>   (defmacro increase ()
>     `(let ((foo (point-max)))
>        (cond
> 	((consp foo)
> 	 (message "consp %s" foo)
> 	 foo)
> 	((numberp foo)
> 	 (1+ fooo))			; Note the misspelling.
> 	(t (message "Something else: %s" foo))))))
>
> (defun call-increase (bar)
>   (cond
>    ((not (or (consp bar)
> 	     (numberp bar)))
>     bar)
>    (t (increase))))
> #########################################################################
>
> Note the misspelling of `foo' as `fooo' on line 10.
>
> (ii) emacs -Q
> (iii) M-x byte-compile-file /path/to/bad-error-position2.el.
> (iv) This gives the warning message:
>
>   bad-error-position2.el:14:4: Warning: reference to free variable `fooo'
>
> (v) The position 14:4 is wrong.  That is the position of the `cond'
> symbol in `call-increase'.
> (vi) The correct message should be:
>
>   bad-error-position2.el:18:8: Warning: reference to free variable `fooo'
>
> ..  18:8 is the position of `increase' on the last line of the file.

Nitpick: one could also argue that the "correct" message should point to
line 8 where is the `fooo` typo.

> +(defun sub-macroexp--posify-form (form call-pos depth)
> +  "Try to apply the transformation of `macroexp--posify-form' to FORM.
> +FORM and CALL-POS are as in that function.  DEPTH is a small integer,
> +decremented at each recursive call, to prevent infinite recursion.
> +Return the changed form, or nil if no change happened."
> +  (let (new-form
> +        )
> +    (cond
> +     ((zerop depth) nil)
> +     ((and (consp form)
> +           (symbolp (car form))
> +           (car form))
> +      (setcar form (position-symbol (car form) call-pos))
> +      form)
> +     ((consp form)
> +      (or (when (setq new-form (sub-macroexp--posify-form
> +                                (car form) call-pos (1- depth)))
> +            (setcar form new-form)
> +            form)
> +          (when (setq new-form (sub-macroexp--posify-form
> +                                (cdr form) call-pos (1- depth)))
> +            (setcdr form new-form)
> +            form)))
> +     ((symbolp form)
> +      (if form                          ; Don't position nil!
> +          (position-symbol form call-pos)))
> +     ((and (or (vectorp form) (recordp form)))
> +      (let ((len (length form))
> +            (i 0)
> +            )
> +        (while (and (< i len)
> +                    (not (setq new-form (sub-macroexp--posify-form
> +                                         (aref form i) call-pos (1- depth)))))
> +          (setq i (1+ i)))
> +        (when (< i len)
> +          (aset form i new-form)
> +          form))))))

That sounds potentially costly  Have you tried to measure the
performance impact in some "typical" cases?

While reading your description I was naively thinking: we can
probably fix it with a trivial patch like:

    diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
    index 19daa57b207..5cc471e32f6 100644
    --- a/lisp/emacs-lisp/macroexp.el
    +++ b/lisp/emacs-lisp/macroexp.el
    @@ -246,6 +246,7 @@ macroexp-macroexpand
       (let* ((macroexpand-all-environment env)
              new-form)
         (while (not (eq form (setq new-form (macroexpand-1 form env))))
    +      (push form byte-compile-form-stack)
           (let ((fun (car-safe form)))
             (setq form
                   (if (and fun (symbolp fun)

Have you tried something like this?
If it doesn't work, do you happen to know why?


        Stefan






  reply	other threads:[~2024-10-11 23:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-10 10:22 bug#73725: Master: Wrong position for byte compiler warning message Alan Mackenzie
2024-10-11 23:45 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-10-12 10:47   ` Alan Mackenzie
2024-10-12 13:53     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-10-12 12:01   ` Mattias Engdegård
2024-10-13 15:31   ` Alan Mackenzie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwvttdii56q.fsf-monnier+emacs@gnu.org \
    --to=bug-gnu-emacs@gnu.org \
    --cc=73725@debbugs.gnu.org \
    --cc=acm@muc.de \
    --cc=mattias.engdegard@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).