bug#50869: 28.0.50; tramp: void-function tramp-file-name-method--cmacro

[Stefan Monnier]

Michael Albinus [2022-04-03 17:44:55] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Note: this is not a compile error.  It's an error that happened during
>> expansion of a compiler-macro.  These are optional, so those errors only
>> affect the performance of the code but shouldn't affect its behavior.
>>
>> IOW this sound more serious than it is.
>>
>> `tramp-file-name-method--cmacro` is supposed to be a function which
>> inlines the code of calls to `tramp-file-name-method`.
>>
>> More specifically `tramp-file-name-method--cmacro` is auto-generated
>> (and put on the `compiler-macro` property of the
>> `tramp-file-name-method` symbol) by the `cl-defsubst` used to define
>> `tramp-file-name-method`.
>>
>> So I guess the source of the error is that unloading `tramp` undefines
>> the functions (both `tramp-file-name-method--cmacro` and
>> `tramp-file-name-method`) but leaves the `compiler-macro` property of the
>> `tramp-file-name-method` symbol.
>>
>> We really should make it so package unloading knows how to undo
>> top-level `put`s.
>
> Honestly, I have no knowledge about the byte-compiler. I've reopened the
> bug, so you can work on it.

The main point is that putting the `cl-defstruct` into the
`tramp-loaddefs.el` is really ugly and I think it's asking for trouble.
In contrast the OP's "errors" aren't actual errors, they're just
messages indicating that some optimization could not be applied.
We should really change the wording to clarify that they're warnings.

(see first hunk in patch below).

But in the mean time I'd much rather live with a harmless
"compiler-macro error" message than with a `cl-defstruct` in
a loaddefs file.

Lars Ingebrigtsen [2022-09-02 13:43:48] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> We really should make it so package unloading knows how to undo
>> top-level `put`s.
> Do you have any ideas for how to do that?

There are a few ways:
- We could let `unload-feature` remove compiler macros when present, as
  in the second hunk of the patch below.
- We could make `cl-defstruct` use `define-symbol-prop` rather than
  `put`, as in the third hunk of the patch below.


        Stefan

diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index c3ba1b36d44..f4df40249de 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -110,7 +110,8 @@ macroexp--compiler-macro
       (let ((symbols-with-pos-enabled t))
         (apply handler form (cdr form)))
     (error
-     (message "Compiler-macro error for %S: Handler: %S\n%S" (car form) 
handler err)
+     (message "Warning: Optimization failure for %S: Handler: %S\n%S"
+              (car form) handler err)
      form)))
 
 (defun macroexp--funcall-if-compiled (_form)
diff --git a/lisp/loadhist.el b/lisp/loadhist.el
index b4ed0432465..76da002314b 100644
--- a/lisp/loadhist.el
+++ b/lisp/loadhist.el
@@ -171,6 +171,8 @@ loadhist-unload-element
     (cond
      ((null hist)
       (defalias fun nil)
+      (if (get fun 'compiler-macro) (put fun 'compiler-macro nil))
+      (if (get fun 'gv-expander)    (put fun 'gv-expander nil))
       ;; Override the change that `defalias' just recorded.
       (put fun 'function-history nil))
      ((equal (car hist) loadhist-unload-filename)
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 80ca43c902a..b7fee7a3487 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3570,7 +3570,7 @@ cl-define-compiler-macro
        (cl-defun ,fname ,(if (memq '&whole args) (delq '&whole args)
                            (cons '_cl-whole-arg args))
          ,@body)
-       (put ',func 'compiler-macro #',fname))))
+       (define-symbol-prop ',func 'compiler-macro #',fname))))
 
 ;;;###autoload
 (defun cl-compiler-macroexpand (form)