master e670412a3e1: Update handling of advices during preload

[Andrea Corallo]

branch: master
commit e670412a3e101e70dc26e021f467faece8cb7f6b
Author: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Commit: Andrea Corallo <acorallo@gnu.org>

    Update handling of advices during preload
    
    * lisp/emacs-lisp/comp-common.el
    (native-comp-never-optimize-functions): Remove macroexpand and
    rename-buffer from default value.
    * lisp/emacs-lisp/comp.el (comp-call-optim-form-call): Document call
    optimization for advised primitives.
    * lisp/emacs-lisp/nadvice.el (advice-add): Remove references to TODOs
    that were completed already earlier.
    * lisp/loadup.el: Disallow advices during preload.  (Bug#67005)
---
 lisp/emacs-lisp/comp-common.el | 9 ++++-----
 lisp/emacs-lisp/comp.el        | 8 ++++++++
 lisp/emacs-lisp/nadvice.el     | 2 --
 lisp/loadup.el                 | 9 +++++++++
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/comp-common.el b/lisp/emacs-lisp/comp-common.el
index 6d94d1bd82e..b7a685223ed 100644
--- a/lisp/emacs-lisp/comp-common.el
+++ b/lisp/emacs-lisp/comp-common.el
@@ -49,11 +49,10 @@ This is intended for debugging the compiler itself.
   :version "28.1")
 
 (defcustom native-comp-never-optimize-functions
-  '(eval
-    ;; The following two are mandatory for Emacs to be working
-    ;; correctly (see comment in `advice--add-function'). DO NOT
-    ;; REMOVE.
-    macroexpand rename-buffer)
+  ;; We used to list those functions here that were advised during
+  ;; preload, but we now prefer to disallow preload advices in
+  ;; loadup.el (bug#67005).
+  '(eval)
   "Primitive functions to exclude from trampoline optimization.
 
 Primitive functions included in this list will not be called
diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 39e32d5142c..3e5ff195764 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -2789,6 +2789,14 @@ FUNCTION can be a function-name or byte compiled 
function."
                     (symbol-function callee)
                   (cl-assert (byte-code-function-p callee))
                   callee))
+             ;; Below call to `subrp' returns nil on an advised
+             ;; primitive F, so that we do not optimize calls to F
+             ;; with the funcall trampoline removal below.  But if F
+             ;; is advised while we compile its call, it is very
+             ;; likely to be advised also when that call is executed.
+             ;; And in that case an "unoptimized" call to F is
+             ;; actually cheaper since it avoids the call to the
+             ;; intermediate native trampoline (bug#67005).
              (subrp (subrp f))
              (comp-func-callee (comp-func-in-unit callee)))
         (cond
diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index 42027c01491..9f2b42f5765 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -509,8 +509,6 @@ HOW can be one of:
 <<>>"
   ;; TODO:
   ;; - record the advice location, to display in describe-function.
-  ;; - change all defadvice in lisp/**/*.el.
-  ;; - obsolete advice.el.
   (let* ((f (symbol-function symbol))
         (nf (advice--normalize symbol f)))
     (unless (eq f nf) (fset symbol nf))
diff --git a/lisp/loadup.el b/lisp/loadup.el
index 07895228d0d..3b58d5fb9b7 100644
--- a/lisp/loadup.el
+++ b/lisp/loadup.el
@@ -393,6 +393,15 @@
 ;; from the repository.  It is generated just after temacs is built.
 (load "leim/leim-list.el" t)
 
+;; Actively disallow advised functions during preload since:
+;; - advices in Emacs's core are generally considered bad style;
+;; - `Snarf-documentation' looses docstrings of primitives advised
+;;   during preload (bug#66032#20).
+(mapatoms
+ (lambda (f)
+   (and (advice--p (symbol-function f))
+        (error "Preload advice on %s" f))))
+
 ;; If you want additional libraries to be preloaded and their
 ;; doc strings kept in the DOC file rather than in core,
 ;; you may load them with a "site-load.el" file.