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#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function Date: Sun, 6 Aug 2023 11:59:36 +0000 Message-ID: References: <8B08E514-B2D5-48F5-BA90-4F5A9492F8F9@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="26453"; mail-complaints-to="usenet@ciao.gmane.io" Cc: acm@muc.de, 65017@debbugs.gnu.org, Eric Marsden To: Stefan Monnier , Mattias =?UTF-8?Q?Engdeg=C3=A5rd?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Aug 06 14:00:18 2023 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 1qScQk-0006n6-BU for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 06 Aug 2023 14:00:18 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qScQY-0001YV-VB; Sun, 06 Aug 2023 08:00:07 -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 1qScQU-0001L2-M9 for bug-gnu-emacs@gnu.org; Sun, 06 Aug 2023 08:00:02 -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 1qScQU-0005RZ-CP for bug-gnu-emacs@gnu.org; Sun, 06 Aug 2023 08:00:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qScQU-00051z-2M for bug-gnu-emacs@gnu.org; Sun, 06 Aug 2023 08:00: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, 06 Aug 2023 12:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65017 X-GNU-PR-Package: emacs Original-Received: via spool by 65017-submit@debbugs.gnu.org id=B65017.169132318719245 (code B ref 65017); Sun, 06 Aug 2023 12:00:02 +0000 Original-Received: (at 65017) by debbugs.gnu.org; 6 Aug 2023 11:59:47 +0000 Original-Received: from localhost ([127.0.0.1]:58751 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qScQE-00050J-KL for submit@debbugs.gnu.org; Sun, 06 Aug 2023 07:59:47 -0400 Original-Received: from mx3.muc.de ([193.149.48.5]:23601) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qScQB-000503-EO for 65017@debbugs.gnu.org; Sun, 06 Aug 2023 07:59:45 -0400 Original-Received: (qmail 76216 invoked by uid 3782); 6 Aug 2023 13:59:37 +0200 Original-Received: from acm.muc.de (pd953a3d8.dip0.t-ipconnect.de [217.83.163.216]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 06 Aug 2023 13:59:36 +0200 Original-Received: (qmail 6500 invoked by uid 1000); 6 Aug 2023 11:59:36 -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:266851 Archived-At: Hello, Stefan and Mattias. On Sat, Aug 05, 2023 at 18:53:48 -0400, Stefan Monnier wrote: > >> I don't know why `symbols-with-pos-enabled` is non-nil at that point (I > >> thought we only enabled it wile byte-compiling), .... > > This is not quite the case. symbols-with-pos-enabled gets erroneously > > bound to t in internal-macroexpand-for-load (emacs-lisp/macroexp.el). > Aha! > > diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el > > index b05aba3e1a7..ea838f5b7b2 100644 > > --- a/lisp/emacs-lisp/macroexp.el > > +++ b/lisp/emacs-lisp/macroexp.el > > @@ -799,8 +799,7 @@ macroexp--debug-eager > > (defun internal-macroexpand-for-load (form full-p) > > ;; Called from the eager-macroexpansion in readevalloop. > > - (let ((symbols-with-pos-enabled t) > > - (print-symbols-bare t)) > > + (let ((print-symbols-bare t)) > > (cond > > ;; Don't repeat the same warning for every top-level element. > > ((eq 'skip (car macroexp--pending-eager-loads)) form) > Looks good to me. AFAICT this binding was added at some point where it > seemed like a good idea but we later figured better places to do it, > and we just didn't remove it because it seemed "harmless" (or because > we just didn't think of it). There is another unneeded binding of symbols-with-pos-enabled in macroexp.el, and several redundant bindings of print-symbols-bare in bytecomp.el (which are commented as such), and one of print-symbols-bare in macroexp.el. The patch below removes these bindings. make bootstrap and make check still work OK, with it. A compile-defun in a function with an error produces the correct error message. I suggest installing this patch into master. > > Stefan, it would still be nice for cl--labels-convert-cache to get > > initialised each time it gets used. > No, the problem is not initialization, as I pointed out. The problem is > that this `eq` should not consider a symbol equal to a sympos *ever* > (contrary to most other uses of `eq` in macros). Are you sure? Why not? If cl--labels-convert-cache is being used inside the byte compiler, it surely needs to consider # and # as eq? There is no mechanism to make these two SWPs eq whilst excluding their eq with the bare symbol. diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index ac040799a22..b9d1948e555 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -489,8 +489,7 @@ byte-compile-recurse-toplevel ;; 3.2.3.1, "Processing of Top Level Forms". The semantics are very ;; subtle: see test/lisp/emacs-lisp/bytecomp-tests.el for interesting ;; cases. - (let ((print-symbols-bare t)) ; Possibly redundant binding. - (setf form (macroexp-macroexpand form byte-compile-macro-environment))) + (setf form (macroexp-macroexpand form byte-compile-macro-environment)) (if (eq (car-safe form) 'progn) (cons (car form) (mapcar (lambda (subform) @@ -568,11 +567,10 @@ byte-compile-initial-macro-environment ;; Don't compile here, since we don't know ;; whether to compile as byte-compile-form ;; or byte-compile-file-form. - (let* ((print-symbols-bare t) ; Possibly redundant binding. - (expanded - (macroexpand--all-toplevel - form - macroexpand-all-environment))) + (let ((expanded + (macroexpand--all-toplevel + form + macroexpand-all-environment))) (eval (byte-run-strip-symbol-positions (bytecomp--copy-tree expanded)) lexical-binding) @@ -2489,8 +2487,7 @@ byte-compile-output-file-form ;; Spill output for the native compiler here (push (make-byte-to-native-top-level :form form :lexical lexical-binding) byte-to-native-top-level-forms)) - (let ((print-symbols-bare t) ; Possibly redundant binding. - (print-escape-newlines t) + (let ((print-escape-newlines t) (print-length nil) (print-level nil) (print-quoted t) @@ -2524,8 +2521,7 @@ byte-compile-output-docform ;; in the input buffer (now current), not in the output buffer. (let ((dynamic-docstrings byte-compile-dynamic-docstrings)) (with-current-buffer byte-compile--outbuffer - (let (position - (print-symbols-bare t)) ; Possibly redundant binding. + (let (position) ;; Insert the doc string, and make it a comment with #@LENGTH. (when (and (>= (nth 1 info) 0) dynamic-docstrings) (setq position (byte-compile-output-as-comment @@ -2621,8 +2617,7 @@ byte-compile-flush-pending byte-compile-jump-tables nil)))) (defun byte-compile-preprocess (form &optional _for-effect) - (let ((print-symbols-bare t)) ; Possibly redundant binding. - (setq form (macroexpand-all form byte-compile-macro-environment))) + (setq form (macroexpand-all form byte-compile-macro-environment)) ;; 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 diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el index b05aba3e1a7..47d663b5d4a 100644 --- a/lisp/emacs-lisp/macroexp.el +++ b/lisp/emacs-lisp/macroexp.el @@ -107,8 +107,7 @@ macroexp--all-clauses (defun macroexp--compiler-macro (handler form) (condition-case-unless-debug err - (let ((symbols-with-pos-enabled t)) - (apply handler form (cdr form))) + (apply handler form (cdr form)) (error (message "Warning: Optimization failure for %S: Handler: %S\n%S" (car form) handler err) @@ -799,40 +798,38 @@ macroexp--debug-eager (defun internal-macroexpand-for-load (form full-p) ;; Called from the eager-macroexpansion in readevalloop. - (let ((symbols-with-pos-enabled t) - (print-symbols-bare t)) - (cond - ;; Don't repeat the same warning for every top-level element. - ((eq 'skip (car macroexp--pending-eager-loads)) form) - ;; If we detect a cycle, skip macro-expansion for now, and output a warning - ;; with a trimmed backtrace. - ((and load-file-name (member load-file-name macroexp--pending-eager-loads)) - (let* ((bt (delq nil - (mapcar #'macroexp--trim-backtrace-frame - (macroexp--backtrace)))) - (elem `(load ,(file-name-nondirectory load-file-name))) - (tail (member elem (cdr (member elem bt))))) - (if tail (setcdr tail (list '…))) - (if (eq (car-safe (car bt)) 'macroexpand-all) (setq bt (cdr bt))) - (if macroexp--debug-eager - (debug 'eager-macroexp-cycle) - (error "Eager macro-expansion skipped due to cycle:\n %s" - (mapconcat #'prin1-to-string (nreverse bt) " => "))) - (push 'skip macroexp--pending-eager-loads) - form)) - (t - (condition-case err - (let ((macroexp--pending-eager-loads - (cons load-file-name macroexp--pending-eager-loads))) - (if full-p - (macroexpand--all-toplevel form) - (macroexpand form))) - (error - ;; Hopefully this shouldn't happen thanks to the cycle detection, - ;; but in case it does happen, let's catch the error and give the - ;; code a chance to macro-expand later. - (error "Eager macro-expansion failure: %S" err) - form)))))) + (cond + ;; Don't repeat the same warning for every top-level element. + ((eq 'skip (car macroexp--pending-eager-loads)) form) + ;; If we detect a cycle, skip macro-expansion for now, and output a warning + ;; with a trimmed backtrace. + ((and load-file-name (member load-file-name macroexp--pending-eager-loads)) + (let* ((bt (delq nil + (mapcar #'macroexp--trim-backtrace-frame + (macroexp--backtrace)))) + (elem `(load ,(file-name-nondirectory load-file-name))) + (tail (member elem (cdr (member elem bt))))) + (if tail (setcdr tail (list '…))) + (if (eq (car-safe (car bt)) 'macroexpand-all) (setq bt (cdr bt))) + (if macroexp--debug-eager + (debug 'eager-macroexp-cycle) + (error "Eager macro-expansion skipped due to cycle:\n %s" + (mapconcat #'prin1-to-string (nreverse bt) " => "))) + (push 'skip macroexp--pending-eager-loads) + form)) + (t + (condition-case err + (let ((macroexp--pending-eager-loads + (cons load-file-name macroexp--pending-eager-loads))) + (if full-p + (macroexpand--all-toplevel form) + (macroexpand form))) + (error + ;; Hopefully this shouldn't happen thanks to the cycle detection, + ;; but in case it does happen, let's catch the error and give the + ;; code a chance to macro-expand later. + (error "Eager macro-expansion failure: %S" err) + form))))) ;; ¡¡¡ Big Ugly Hack !!! ;; src/bootstrap-emacs is mostly used to compile .el files, so it needs > Stefan -- Alan Mackenzie (Nuremberg, Germany).