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: Sun, 13 Oct 2024 15:31:55 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40807"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 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 Sun Oct 13 17:33:12 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 1t00al-000ANx-G6 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 13 Oct 2024 17:33:11 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t00aR-0005KX-PM; Sun, 13 Oct 2024 11:32:51 -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 1t00aN-0005Jh-GS for bug-gnu-emacs@gnu.org; Sun, 13 Oct 2024 11:32:47 -0400 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 1t00aN-0000gc-8N for bug-gnu-emacs@gnu.org; Sun, 13 Oct 2024 11:32:47 -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=awOrR1kDYiWSTEwgMB+kOneoxDBb6YpSW0qalRlJSH8=; b=fIgXPpEB88/n1hIzADo6APaOU1v2HQsDhKDhuLbTuj/aayYROOi6z0JNiuWbYHZWFa6Ah5tb7+8kI9GgDvbpU57H5yyu7zJRGHUc4xADkSvKy5Q0vKlL0iV14wqB7YqRF1QxQ5W3hT48I7bJ93fj+p5auGmBZSGPAJesAW3Wqab8S5of34Z3js6OzcyDcPTse42rPwjVPzxSVt89/cLb/WBfNHOaWxAgeE8N0+gUB41CB6ge0SbmKVm3oT4fu2D678CMSAsqFLQFuhiikL/IFJ2gtOTHN8ghI7QLzybY38T60j4SiYuhIG6r7QUH2X2eAsPgOLwyaZgW8numD1o4QQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t00ac-000175-Qt for bug-gnu-emacs@gnu.org; Sun, 13 Oct 2024 11:33: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: Sun, 13 Oct 2024 15:33: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.17288335444212 (code B ref 73725); Sun, 13 Oct 2024 15:33:02 +0000 Original-Received: (at 73725) by debbugs.gnu.org; 13 Oct 2024 15:32:24 +0000 Original-Received: from localhost ([127.0.0.1]:52111 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t00a0-00015n-9A for submit@debbugs.gnu.org; Sun, 13 Oct 2024 11:32:24 -0400 Original-Received: from mail.muc.de ([193.149.48.3]:18718) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t00Zv-00015N-Fy for 73725@debbugs.gnu.org; Sun, 13 Oct 2024 11:32:22 -0400 Original-Received: (qmail 19334 invoked by uid 3782); 13 Oct 2024 17:31:56 +0200 Original-Received: from muc.de (p4fe15e31.dip0.t-ipconnect.de [79.225.94.49]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 13 Oct 2024 17:31:56 +0200 Original-Received: (qmail 1875 invoked by uid 1000); 13 Oct 2024 15:31:55 -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:293528 Archived-At: Hello again, Stefan. On Fri, Oct 11, 2024 at 19:45:18 -0400, Stefan Monnier wrote: > > (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. In this case, no. The function from which the macro is called might have a locally bound variable called `fooo'. ;-) But there are surely errors in macros which aren't like that which might be picked up by compiling the macro. Then warnings containing the line in the macro could be given. That would be a non-trivial exercise. > > +(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? I've fixed this bug together with bug#73746, and submitted the patch for that other bug which fixes this bug, too. I've just done $ time make -j25 bootstrap on a branch with the bug#73746 patch, and a (more or less) standard master. The bug#73746 branch took 1min 58sec, the standard branch to 1min 51sec. That's a difference of ~6% in the bootstrap. It will of course be a bigger difference in the compilation. If that was felt to be too much, it would be possible instead to go round macroexp--expand-all and byte-optimize-form-code-walker and all the functions like byte-optimize-letX, putting in custom `position-symbol' calls. This would be a lot of work, though. > 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? Like you said in another post, it might well not work for the byte-opt.el functionality. I think I was looking around for that sort of solution, and decided it wouldn't work, for a reason I've forgotten, before coming up with the idea in my patch > Stefan -- Alan Mackenzie (Nuremberg, Germany).