From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#53526: 29.0.50; macroexp-warn-and-return API change Date: Tue, 25 Jan 2022 14:10:12 -0500 Message-ID: References: Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="557"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 53526@debbugs.gnu.org To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Jan 25 20:18:46 2022 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 1nCRL3-000AVi-Tw for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 25 Jan 2022 20:18:46 +0100 Original-Received: from localhost ([::1]:43548 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nCRKz-0001OG-CU for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 25 Jan 2022 14:18:45 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:32904) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nCRDa-0005IV-Ve for bug-gnu-emacs@gnu.org; Tue, 25 Jan 2022 14:11:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57209) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nCRDa-0006pM-JR for bug-gnu-emacs@gnu.org; Tue, 25 Jan 2022 14:11:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nCRDa-0000si-Ak for bug-gnu-emacs@gnu.org; Tue, 25 Jan 2022 14:11:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 25 Jan 2022 19:11:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 53526 X-GNU-PR-Package: emacs Original-Received: via spool by 53526-submit@debbugs.gnu.org id=B53526.16431378393355 (code B ref 53526); Tue, 25 Jan 2022 19:11:02 +0000 Original-Received: (at 53526) by debbugs.gnu.org; 25 Jan 2022 19:10:39 +0000 Original-Received: from localhost ([127.0.0.1]:50112 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nCRDD-0000s2-0l for submit@debbugs.gnu.org; Tue, 25 Jan 2022 14:10:39 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:51362) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nCRD7-0000rj-GM for 53526@debbugs.gnu.org; Tue, 25 Jan 2022 14:10:37 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 6DEC84428C6; Tue, 25 Jan 2022 14:10:27 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 7A9E74428C0; Tue, 25 Jan 2022 14:10:25 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1643137825; bh=oL91isvRWsT3BaZwCeQPj7sI0QI8g4SPhOSh3hNY/pA=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=o9XpKIxhzIfjC2Fq7YG5e2XdnPD0oAoGDYcx7gNIY677TsNS1BuPie7nIgs4pOLLs Ar9GHbURCb9WD2Knh7iHSqAblYupgSL3kcvg6n3GCbldbvQO7/lhTYvDWcNW3dI9Gm Eo95H47vg9wes0w/UfyFsEZs59C0cu3pQosxLxVD5vN1Jrz6BsoIAehkE6iJWyan5y dkEuSWfrhsGnf1FAawalknCzJPHT99AOY8glgSksVZwumlgMFITwhZRYRuowqYmXgm IzJTQiaFUfLVXAujxpsY256Jiee3AX+1hCCKKwSuJs10qsUq0MlZAeH8PHnS0hDB10 yTtRrX/ncfDKw== Original-Received: from pastel (unknown [216.154.30.173]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 3860312098D; Tue, 25 Jan 2022 14:10:25 -0500 (EST) In-Reply-To: (Alan Mackenzie's message of "Tue, 25 Jan 2022 18:16:56 +0000") 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" Xref: news.gmane.io gmane.emacs.bugs:225216 Archived-At: >> -(defun macroexp-warn-and-return (msg form &optional category compile-only) >> +(defun macroexp-warn-and-return (arg msg form &optional category compile-only) > No, it isn't. All the uses of the function are in lisp/emacs-lisp, and > I understood the function to be an internal one. No, its name was changed from "macroexp--" to "macroexp-" in Emacs-28, specifically to make available for third party packages. It was announced in etc/NEWS, for example. While `bindat.el` lives in `lisp/emacs-lisp`, it's an example of a non-core package that benefits from it. >> I suspect that the `arg` should be added at the end instead. > The other functions (like byte-compile-warn-x) which have acquired this > extra argument need to have it at the start, since there are an > indeterminate number of &rest args going into a `format'. So it seemed > better just to do the same with this function, to preserve a sort of > compatibility. While I can see the value of this aesthetic argument, I think breaking backward compatibility was a published API is a more serious problem. On the upside, moving it to the end will make it optional, which is good since in many cases we can use the `form` argument instead (which `byte-compile-warn-x` doesn't have). >> While I'm here I also noticed that `byte-compile-form-stack` is a poor >> name for a variable declared in `macroexp.el`. > > It's an integral part of bytecomp.el. It got moved to macroexp.el > because it is used (twice) there, and that file is loaded into bootstrap > emacs before bytecomp.el. > > There is precedent for this "mis"naming, namely > byte-compile-bound-variables. `byte-compile-bound-variables` is defined in `bytecomp.el`, not in `macroexp.el`. And indeed, `byte-compile-bound-variables` is only set/modified by the byte compiler, so it really belongs there. I can see that just moving the definition to bytecomp.el and using (defvar byte-compile-form-stack) in macroexp.el won't work because the `push` requires the var to have a value. Still, the current setup is really ugly: that var belongs in `bytecomp.el`. > It started off life with a double hyphen in bytecomp.el. But when it > started getting used in macroexp.el (during the expansion of a macro) it > lost the extra hyphen and got moved there. I'd put a double hyphen there simply because it's not something that we want to expose as an official API. Just because the bytecompiler's macroexpansion phase is implemented in a separate file doesn't justify making the var public. > It's no big deal, I think, just that there's no completely neat way of > doing this, so the compromise actually used is pretty arbitrary. > If the variable were in bytecomp.el, we'd probably need a boundp call > in the two places we use it in macroexp.el. It at least deserves a prominent comment explaining why it's there. > Whilst on the topic of macroexp-warn-and-return (and > macroexp--wrap-warn), I have to admit having difficulty understanding > these functions, both how they work and what they're for. > > My impression up till a couple of days ago was that they were ways of > coping with the old warning position mechanism, and were intended to > compensate for its deficiencies. The original motivation was indeed to improve the error messages by including more relevant line information. This part was made largely irrelevant with your patch. But it's still relevant because macros can use it without being tied to the byte-compiler. Also a nice side-effect is that the warnings are emitted (mostly) in the order they appear in the code, whereas otherwise we'd first have the warnings emitted during macroexpansion, then warnings emitted during the compilation. > Now, I'm much less sure. Was I indeed mistaken? If I was, what then is > the purpose of these functions, which defer warning messages in some > fashion? If I was right, it would be a good thing to dismantle them, > since they are complicated and difficult, and no longer needed. As I > said, I don't really understand them. I don't see what's difficult about it: it lets you attach a warning to a piece of code. Stefan