bug#64494: 30.0.50; Recursive load error with native-compile

bug#64494: 30.0.50; Recursive load error with native-compile

[German Pacenza]

Steps to reproduce:
clean native compile cache
emacs -Q
C-x r b (bookmark-jump)

Error message appears:
emacs-lisp-compilation-mode: Recursive load: "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/cl-lib.elc", "/home/german/repos/emacs/lisp/emacs-
 lisp/pp.elc", "/home/german/repos/emacs/lisp/bookmark.elc"

Reverting commit 40492581f96626e405e4b453456b8c9b83822c97 fixes the issue.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-07-06 built on KRONOS
Repository revision: 40492581f96626e405e4b453456b8c9b83822c97
Repository branch: master
System Description: Manjaro Linux

Configured using:
 'configure --with-pgtk --with-native-compilation --without-libsystemd
 --without-compress-install --prefix=/home/german/.local/emacs'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PGTK PNG
RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER
WEBP XIM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: es_AR.UTF-8
  value of $LC_NUMERIC: es_AR.UTF-8
  value of $LC_TIME: es_AR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  vertico-mode: t
  savehist-mode: t
  popper-mode: t
  minibuffer-depth-indicate-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  window-divider-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/german/.emacs.d/elpa/embark-consult-0.7/embark-org hides /home/german/.emacs.d/elpa/embark-0.22.1/embark-org
/home/german/.emacs.d/elpa/transient-0.4.1/transient hides /home/german/.local/emacs/share/emacs/30.0.50/lisp/transient

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils compile text-property-search
comint ansi-osc ansi-color ring comp comp-cstr rx vertico savehist
popper subr-x transient cl-extra help-mode cl-seq format-spec edmacro
kmacro eieio byte-opt bytecomp byte-compile eieio-core cl-macs gv compat
mb-depth g3r-light-theme info autothemer-autoloads doom-themes-autoloads
ef-themes-autoloads embark-consult-autoloads consult-autoloads
embark-autoloads fontify-face-autoloads kurecolor-autoloads
magit-autoloads git-commit-autoloads magit-section-autoloads
dash-autoloads orderless-autoloads popper-autoloads
rainbow-mode-autoloads s-autoloads transient-autoloads vertico-autoloads
with-editor-autoloads compat-autoloads yuck-mode-autoloads warnings
icons cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/pgtk-win pgtk-win term/common-win pgtk-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo gtk pgtk
lcms2 multi-tty move-toolbar make-network-process native-compile emacs)

Memory information:
((conses 16 129884 100287) (symbols 48 10014 5)
 (strings 32 32148 18103) (string-bytes 1 1081003) (vectors 16 21625)
 (vector-slots 8 412849 137704) (floats 8 58 241)
 (intervals 56 476 67) (buffers 984 11))

-- 
German Pacenza

bug#64494: 30.0.50; Recursive load error with native-compile

[Eli Zaretskii]

> From: German Pacenza <germanp82@hotmail.com>
> Date: Thu, 06 Jul 2023 08:45:10 -0300
> 
> 
> Steps to reproduce:
> clean native compile cache
> emacs -Q
> C-x r b (bookmark-jump)
> 
> Error message appears:
> emacs-lisp-compilation-mode: Recursive load: "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/ring.elc", "/home/german/repos/emacs/lisp/comint.elc", "/home/german/repos/emacs/lisp/progmodes/compile.elc", "/home/german/repos/emacs/lisp/emacs-lisp/cl-lib.elc", "/home/german/repos/emacs/lisp/emac
 s-lisp/pp.elc", "/home/german/repos/emacs/lisp/bookmark.elc"
> 
> Reverting commit 40492581f96626e405e4b453456b8c9b83822c97 fixes the issue.

Thanks, I've reverted that commit for now.

Andrea, Stefan: any ideas for how to allow emacs-lisp-compilation-mode
in async compilation buffers without triggering this recursive-load
nastiness?

bug#64494: 30.0.50; Recursive load error with native-compile

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Andrea, Stefan: any ideas for how to allow emacs-lisp-compilation-mode
> in async compilation buffers without triggering this recursive-load
> nastiness?

I'm wondering if instead of having the C code do:

    /* This is so deferred compilation is able to compile comp
       dependencies breaking circularity.  */
    if (comp__compilable)
      {
        /* Startup is done, comp is usable.  */
        CALL0I (startup--require-comp-safely);
        CALLN (Ffuncall, intern_c_string ("native--compile-async"),
               src, Qnil, Qlate);
      }
    else
      Vcomp__delayed_sources = Fcons (src, Vcomp__delayed_sources);

we shouldn't just do something like

    pending_funcalls
        = Fcons (list (Qnative__compile_async, src, Qnil, Qlate),
                 pending_funcalls);

I.e. never call native--compile-async synchronously.
The patch below seems to work so far, tho I haven't tried bootstrapping yet.


        Stefan


diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 77584b692a4..4892733d456 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -4230,6 +4230,7 @@ native-compile-async-skip-p
                       (string-match-p re file))
                     native-comp-jit-compilation-deny-list))))
 
+;;;###autoload
 (defun native--compile-async (files &optional recursively load selector)
   ;; BEWARE, this function is also called directly from C.
   "Compile FILES asynchronously.
diff --git a/lisp/startup.el b/lisp/startup.el
index 5a389294e78..7f601668369 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -520,27 +520,6 @@ startup--xdg-or-homedot
       xdg-dir)
      (t emacs-d-dir))))
 
-(defvar comp--compilable)
-(defvar comp--delayed-sources)
-(defun startup--require-comp-safely ()
-  "Require the native compiler avoiding circular dependencies."
-  (when (featurep 'native-compile)
-    ;; Require comp with `comp--compilable' set to nil to break
-    ;; circularity.
-    (let ((comp--compilable nil))
-      (require 'comp))
-    (native--compile-async comp--delayed-sources nil 'late)
-    (setq comp--delayed-sources nil)))
-
-(declare-function native--compile-async "comp.el"
-                  (files &optional recursively load selector))
-(defun startup--honor-delayed-native-compilations ()
-  "Honor pending delayed deferred native compilations."
-  (when (and (native-comp-available-p)
-             comp--delayed-sources)
-    (startup--require-comp-safely))
-  (setq comp--compilable t))
-
 (defvar native-comp-eln-load-path)
 (defvar native-comp-jit-compilation)
 (defvar native-comp-enable-subr-trampolines)
@@ -846,8 +825,7 @@ normal-top-level
                                 nil)))
             (setq env (cdr env)))))
       (when display
-        (setq process-environment (delete display process-environment)))))
-  (startup--honor-delayed-native-compilations))
+        (setq process-environment (delete display process-environment))))))
 
 ;; Precompute the keyboard equivalents in the menu bar items.
 ;; Command-line options supported by tty's:
diff --git a/src/comp.c b/src/comp.c
index 013ac6358c1..003bec38efa 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -5199,17 +5199,9 @@ maybe_defer_native_compilation (Lisp_Object function_name,
 
   Fputhash (function_name, definition, Vcomp_deferred_pending_h);
 
-  /* This is so deferred compilation is able to compile comp
-     dependencies breaking circularity.  */
-  if (comp__compilable)
-    {
-      /* Startup is done, comp is usable.  */
-      CALL0I (startup--require-comp-safely);
-      CALLN (Ffuncall, intern_c_string ("native--compile-async"),
-	     src, Qnil, Qlate);
-    }
-  else
-    Vcomp__delayed_sources = Fcons (src, Vcomp__delayed_sources);
+  pending_funcalls
+    = Fcons (list (Qnative__compile_async, src, Qnil, Qlate),
+             pending_funcalls);
 }
 
 \f
@@ -5798,6 +5790,8 @@ syms_of_comp (void)
         build_pure_c_string ("eln file inconsistent with current runtime "
 			     "configuration, please recompile"));
 
+  DEFSYM (Qnative__compile_async, "native--compile-async");
+  
   defsubr (&Scomp__subr_signature);
   defsubr (&Scomp_el_to_eln_rel_filename);
   defsubr (&Scomp_el_to_eln_filename);

bug#64494: 30.0.50; Recursive load error with native-compile

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: German Pacenza <germanp82@hotmail.com>,  Andrea Corallo
>  <acorallo@gnu.org>,  64494@debbugs.gnu.org
> Date: Fri, 07 Jul 2023 13:02:55 -0400
> 
> The patch below seems to work so far, tho I haven't tried bootstrapping yet.

Thanks, but does it allow to put back the change I reverted?  It isn't
in the patch, so I'm not sure if you tried that.

bug#64494: 30.0.50; Recursive load error with native-compile

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Thanks, but does it allow to put back the change I reverted?

I believe so, yes.

> It isn't in the patch, so I'm not sure if you tried that.

The patch was tested against commit 40492581f9 (i.e. the commit just
before the reversion).

The way it works is that instead of having the C code call
`native--compile-async` (which loads `comp.el`) right in the middle of
loading another file, it delays the call to the next time Emacs waits.
After all `native--compile-async` doesn't do anything immediately urgent
since all it does is schedule a future compilation.

There's another thing I don't quite understand about our code here: why
do we call `native--compile-async` from `defalias` (via
`maybe_defer_native_compilation`) rather than doing it from something
like `after-load-functions`?


        Stefan

bug#64494: 30.0.50; Recursive load error with native-compile

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> There's another thing I don't quite understand about our code here: why
> do we call `native--compile-async` from `defalias` (via
> `maybe_defer_native_compilation`) rather than doing it from something
> like `after-load-functions`?

Hmm... I see that its not done for the "deferred compilation" but it's
done for the subsequent *reload* (which replaces the previously loaded
bytecode functions with their newly native-compiled version).

The problem being that we shouldn't load the .eln (and thus replace
the bytecode with native code) if the bytecode has been replaced with
something else in the mean time.

IOW, we need some way to detect when changes occur to the `.elc`-defined
functions between the time the `.elc` file is loaded and the time the
`.eln` file is available for load.  This is done by storing the `.elc`
definitions in an auxiliary hash-table (`comp-deferred-pending-h`) so
they can be compared to the current definition before replacing it withe
new definition from`.eln`.

So, if we want to do it from `after-load-functions`, we need to use
`load-history` to collect all the definitions and populate
`comp-deferred-pending-h`.


        Stefan

bug#64494: 30.0.50; Recursive load error with native-compile

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> we shouldn't just do something like
>
>     pending_funcalls
>         = Fcons (list (Qnative__compile_async, src, Qnil, Qlate),
>                  pending_funcalls);
>
> I.e. never call native--compile-async synchronously.
> The patch below seems to work so far, tho I haven't tried bootstrapping yet.

After further tests, I pushed it to `master`, along with the
re-installation of commit 140492581f96.


        Stefan