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

Previous Next

Package: emacs;

Reported by: Alan Mackenzie <acm <at> muc.de>

Date: Thu, 10 Oct 2024 10:24:02 UTC

Severity: normal

Tags: patch

Full log


Message #8 received at 73725 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: Mattias EngdegÄrd <mattias.engdegard <at> gmail.com>,
 73725 <at> debbugs.gnu.org
Subject: Re: bug#73725: Master: Wrong position for byte compiler warning
 message.
Date: Fri, 11 Oct 2024 19:45:18 -0400
> (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





This bug report was last modified 106 days ago.

Previous Next


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