[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#65017: 29.1; Byte compiler interaction with cl-lib function objects,
From: |
Alan Mackenzie |
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 |
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 #<symbol foo at
42> and #<symbol foo at 60> 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).
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, (continued)
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Eli Zaretskii, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Eli Zaretskii, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Eli Zaretskii, 2023/08/04
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/05
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/05
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function,
Alan Mackenzie <=
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/07
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/08
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/09
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/10
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/11
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Mattias Engdegård, 2023/08/12
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/12
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Mattias Engdegård, 2023/08/12
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Alan Mackenzie, 2023/08/12
- bug#65017: 29.1; Byte compiler interaction with cl-lib function objects, removes symbol-function, Stefan Monnier, 2023/08/12