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: Thu, 10 Oct 2024 10:22:49 +0000 Message-ID: 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="8163"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Stefan Monnier , Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= To: 73725@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Oct 10 12:24:14 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 1syqL7-0001yt-SR for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 10 Oct 2024 12:24:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1syqKp-0003SA-Tr; Thu, 10 Oct 2024 06:23:55 -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 1syqKn-0003Rz-4r for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 06:23:53 -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 1syqKm-0003wO-SB for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 06:23:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=From:MIME-Version:Date:To:Subject; bh=HILVabeeeWPtM8/0+IcPjs+JLZtdO4RM0gzT4xQxiag=; b=tGK/ZFql0OnMSEfZQlkUFRxdUTdvoB67+k0qtC8KhnG0tVT34bsy+BRt4IHjnR6QJLD2O2mA5YH0Blurx/Zl8EYsV4GVJhK/9JyCpm8k8J1Ai2iJzDo8sxlG8woxB8cXqaijFJuXT+uOzrAKBBO1z7l/FHQPDc73MFbtcBjL6ytG68CeZmfSJ9WN0gDonl8U4BlfSm0YBuABH2hWe19ItELuV/GR1njhK5GQZ4mlqjTeUoOu3XRtvfWByUI21B7g6guJU/zi62aCLPBdNGKJhuGG69ofkNRc8ZyW9j6yJWdWfvfCs7qyWUGBvRdzseroR0w/T39hjDuYxroK2cZAIg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1syqKw-0000Jo-BG; Thu, 10 Oct 2024 06:24:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: monnier@iro.umontreal.ca, mattias.engdegard@gmail.com, bug-gnu-emacs@gnu.org Resent-Date: Thu, 10 Oct 2024 10:24:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 73725 X-GNU-PR-Package: emacs X-Debbugs-Original-To: bug-gnu-emacs@gnu.org X-Debbugs-Original-Xcc: Stefan Monnier , Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= Original-Received: via spool by submit@debbugs.gnu.org id=B.17285558001156 (code B ref -1); Thu, 10 Oct 2024 10:24:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 10 Oct 2024 10:23:20 +0000 Original-Received: from localhost ([127.0.0.1]:58764 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1syqKF-0000IZ-O2 for submit@debbugs.gnu.org; Thu, 10 Oct 2024 06:23:20 -0400 Original-Received: from lists.gnu.org ([209.51.188.17]:45804) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1syqKC-0000IR-No for submit@debbugs.gnu.org; Thu, 10 Oct 2024 06:23:18 -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 1syqK1-0003Ju-87 for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 06:23:05 -0400 Original-Received: from mail.muc.de ([193.149.48.3]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1syqJy-0003oh-Tw for bug-gnu-emacs@gnu.org; Thu, 10 Oct 2024 06:23:05 -0400 Original-Received: (qmail 23986 invoked by uid 3782); 10 Oct 2024 12:22:50 +0200 Original-Received: from muc.de (p4fe15213.dip0.t-ipconnect.de [79.225.82.19]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Thu, 10 Oct 2024 12:22:50 +0200 Original-Received: (qmail 11006 invoked by uid 1000); 10 Oct 2024 10:22:49 -0000 Content-Disposition: inline X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de Received-SPF: pass client-ip=193.149.48.3; envelope-from=acm@muc.de; helo=mail.muc.de X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action 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:293266 Archived-At: Hello, Emacs. On the master branch. (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. ######################################################################### Diagnosis --------- When the byte compiler expands the invocation of `increase' on the last line, `increase' is a symbol with position. The form returned by the macro expander, however, has no position on its first symbol, the `let'. Thus when the warning is being processed, the pertinent entry on byte-compile-form-stack has no symbols with position, so the code takes a SWP from a deeper nested form, the `cond' form on line 14, and derives the warning position from it. ######################################################################### Proposed fix ------------ To fix this I propose amending the macro expander, such that if the first symbol in a form being expanded is a SWP, the position is applied to the expanded form, typically on the car of that form, but possibly elsewhere if that expanded form isn't a list. The following patch appears to fix the bug: diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 29e7882c851..e0a59d19d4e 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -2582,14 +2582,17 @@ byte-compile-flush-pending byte-compile-jump-tables nil)))) (defun byte-compile-preprocess (form &optional _for-effect) - (setq form (macroexpand-all form byte-compile-macro-environment)) + (let ((call-pos (and (consp form) + (symbol-with-pos-p (car form)) + (symbol-with-pos-pos (car form))))) + (setq form (macroexpand-all form byte-compile-macro-environment call-pos)) ;; FIXME: We should run byte-optimize-form here, but it currently does not ;; recurse through all the code, so we'd have to fix this first. ;; Maybe a good fix would be to merge byte-optimize-form into ;; macroexpand-all. ;; (if (memq byte-optimize '(t source)) ;; (setq form (byte-optimize-form form for-effect))) - (cconv-closure-convert form byte-compile-bound-variables)) + (cconv-closure-convert form byte-compile-bound-variables))) ;; byte-hunk-handlers cannot call this! (defun byte-compile-toplevel-file-form (top-level-form) diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el index 4524eccc7ef..44d49bd7757 100644 --- a/lisp/emacs-lisp/macroexp.el +++ b/lisp/emacs-lisp/macroexp.el @@ -237,11 +237,63 @@ macroexpand-1 form)))))))) (t form))) +(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)))))) + +(defun macroexp--posify-form (form call-pos) + "Try to apply the position CALL-POS to the form FORM. +CALL-POS is a buffer position, a number. FORM may be any lisp form, +and is typically the output form returned by macro expansion. +Apply CALL-POS to FORM as a symbol with position, such that +`byte-compile--first-symbol-with-pos' can later return it. Return +the possibly modified FORM." + (let ((new-form (sub-macroexp--posify-form form call-pos 10))) + (or new-form form))) + (defun macroexp-macroexpand (form env) "Like `macroexpand' but checking obsolescence." (let* ((macroexpand-all-environment env) - new-form) + (call-pos (and (consp form) + (symbol-with-pos-p (car form)) + (symbol-with-pos-pos (car form)))) + macroexpanded new-form) (while (not (eq form (setq new-form (macroexpand-1 form env)))) + (setq macroexpanded t) (let ((fun (car-safe form))) (setq form (if (and fun (symbolp fun) @@ -252,6 +304,8 @@ macroexp-macroexpand (if (symbolp (symbol-function fun)) "alias" "macro")) new-form (list 'obsolete fun) nil fun) new-form)))) + (when (and macroexpanded call-pos) + (setq form (macroexp--posify-form form call-pos))) form)) (defun macroexp--unfold-lambda (form &optional name) @@ -516,14 +570,22 @@ macroexp--expand-all (_ form)))))) ;;;###autoload -(defun macroexpand-all (form &optional environment) +(defun macroexpand-all (form &optional environment call-pos) "Return result of expanding macros at all levels in FORM. If no macros are expanded, FORM is returned unchanged. The second optional arg ENVIRONMENT specifies an environment of macro -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))) +definitions to shadow the loaded ones for use in file byte-compilation. +CALL-POS, if non-nil, is a buffer position which will be incorporated +into the result form as a symbol with position, later to be usable by +`byte-compile--warning-source-offset'." + (let* + ((macroexpand-all-environment environment) + (macroexp--dynvars macroexp--dynvars) + (new-form + (macroexp--expand-all form))) + (if call-pos + (macroexp--posify-form new-form call-pos) + new-form))) ;; This function is like `macroexpand-all' but for use with top-level ;; forms. It does not dynbind `macroexp--dynvars' because we want -- Alan Mackenzie (Nuremberg, Germany).