eval-when-compile help?

eval-when-compile help?

[Dmitry Gutov]

Hi all,

I need to add a piece of code to xref.el which will make it load eieio 
and define a class xref-location when [possibly byte-compiled and] 
loaded in Emacs 27 or older.

But preferably avoid loading EIEIO in Emacs 28+. Both at runtime when 
xref.el is loaded, and when it's compiled too.

The current patch is below. Am I overdoing it with 'eval-and-compile'?

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 1feb6ef915..c9334cfa04 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -74,6 +74,13 @@
  (require 'ring)
  (require 'project)

+(when (version< emacs-version "28")
+  (eval-and-compile
+    (require 'eieio)
+
+    (defclass xref-location () ()
+      :documentation "(Obsolete) location represents a position in a 
file or buffer.")))
+
  (defgroup xref nil "Cross-referencing commands."
    :version "25.1"
    :group 'tools)

Re: eval-when-compile help?

[Stefan Monnier]

> +(when (version< emacs-version "28")
> +  (eval-and-compile
> +    (require 'eieio)
> +
> +    (defclass xref-location () ()
> +      :documentation "(Obsolete) location represents a position in a file or buffer.")))
[...]
> Am I overdoing it with 'eval-and-compile'?

I suspect that "from the outside" you don't need the `eval-and-compile`
at all here.  But if you remove it, there's a good chance that
compilation of `defclass` will fail (because the `require` won't be
executed at compile-time).
This said, I'd recommend you hoist the `eval-and-compile` outside of the
`when` so that `eieio` is not loaded in Emacs≥28.


        Stefan

Re: eval-when-compile help?

[Dmitry Gutov]

On 05.10.2021 15:19, Stefan Monnier wrote:
>> +(when (version< emacs-version "28")
>> +  (eval-and-compile
>> +    (require 'eieio)
>> +
>> +    (defclass xref-location () ()
>> +      :documentation "(Obsolete) location represents a position in a file or buffer.")))
> [...]
>> Am I overdoing it with 'eval-and-compile'?
> 
> I suspect that "from the outside" you don't need the `eval-and-compile`
> at all here.  But if you remove it, there's a good chance that
> compilation of `defclass` will fail (because the `require` won't be
> executed at compile-time).
> This said, I'd recommend you hoist the `eval-and-compile` outside of the
> `when` so that `eieio` is not loaded in Emacs≥28.

In my testing, this doesn't make `eieio` loaded in Emacs 28 at runtime. 
Though I suppose it gets loaded during byte-compilation.

Moving `eval-and-compile` to the top level makes a lot of sense, but now 
I get the compilation warnings back (when compiling in Emacs 28).

So I suppose this is the final form:

(eval-and-compile
   (when (version< emacs-version "28")
     (require 'eieio)

     (with-no-warnings
       (defclass xref-location () ()
         :documentation "(Obsolete) location represents a position in a 
file or buffer."))))

Re: eval-when-compile help?

[Stefan Monnier]

> So I suppose this is the final form:
>
> (eval-and-compile
>   (when (version< emacs-version "28")
>     (require 'eieio)
>
>     (with-no-warnings
>       (defclass xref-location () ()
>         :documentation "(Obsolete) location represents a position in a file
>        or buffer."))))

Better use `with-suppressed-warnings` than `with-no-warnings` (or at
least add a comment explaining what warning you're silencing).

BTW, maybe a better option is to use a macro like:

    (defmacro if-when-compile (test then else)
      (if (eval test t) then else))

and then do

    (if-when-compile (version< emacs-version "28")
        (progn
          (require 'eieio)
          
          (defclass xref-location () ()
            :documentation
            "(Obsolete) location represents a position in a file or buffer.")))


-- Stefan

Re: eval-when-compile help?

[Dmitry Gutov]

On 05.10.2021 18:05, Stefan Monnier wrote:
>> So I suppose this is the final form:
>>
>> (eval-and-compile
>>    (when (version< emacs-version "28")
>>      (require 'eieio)
>>
>>      (with-no-warnings
>>        (defclass xref-location () ()
>>          :documentation "(Obsolete) location represents a position in a file
>>         or buffer."))))
> 
> Better use `with-suppressed-warnings` than `with-no-warnings` (or at
> least add a comment explaining what warning you're silencing).

with-suppressed-warnings was added in 27, and xref.el is still targeting 
26+. A comment sounds fine.

> BTW, maybe a better option is to use a macro like:
> 
>      (defmacro if-when-compile (test then else)
>        (if (eval test t) then else))
> 
> and then do
> 
>      (if-when-compile (version< emacs-version "28")
>          (progn
>            (require 'eieio)
>            
>            (defclass xref-location () ()
>              :documentation
>              "(Obsolete) location represents a position in a file or buffer.")))

That gives me

   Wrong number of arguments: (3 . 3), 2

What would I put in the 'else' branch anyway?

Re: eval-when-compile help?

[Stefan Monnier]

>> BTW, maybe a better option is to use a macro like:
>>      (defmacro if-when-compile (test then else)
>>        (if (eval test t) then else))
>> and then do
>>      (if-when-compile (version< emacs-version "28")
>>          (progn
>>            (require 'eieio)
>>                       (defclass xref-location () ()
>>              :documentation
>>              "(Obsolete) location represents a position in a file or buffer.")))
>
> That gives me
>
>   Wrong number of arguments: (3 . 3), 2

;-)

> What would I put in the 'else' branch anyway?

I was thinking of a generic `if-when-compile` which wouldn't be specific
to this use (and `when-when-compile` sounds just odd ;-).


        Stefan