unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* eval-when-compile help?
@ 2021-10-05  3:01 Dmitry Gutov
  2021-10-05 12:19 ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Gutov @ 2021-10-05  3:01 UTC (permalink / raw)
  To: emacs-devel

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)



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: eval-when-compile help?
  2021-10-05  3:01 eval-when-compile help? Dmitry Gutov
@ 2021-10-05 12:19 ` Stefan Monnier
  2021-10-05 14:16   ` Dmitry Gutov
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2021-10-05 12:19 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

> +(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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: eval-when-compile help?
  2021-10-05 12:19 ` Stefan Monnier
@ 2021-10-05 14:16   ` Dmitry Gutov
  2021-10-05 15:05     ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Gutov @ 2021-10-05 14:16 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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."))))



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: eval-when-compile help?
  2021-10-05 14:16   ` Dmitry Gutov
@ 2021-10-05 15:05     ` Stefan Monnier
  2021-10-05 16:22       ` Dmitry Gutov
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2021-10-05 15:05 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

> 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




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: eval-when-compile help?
  2021-10-05 15:05     ` Stefan Monnier
@ 2021-10-05 16:22       ` Dmitry Gutov
  2021-10-05 19:53         ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Gutov @ 2021-10-05 16:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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?



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: eval-when-compile help?
  2021-10-05 16:22       ` Dmitry Gutov
@ 2021-10-05 19:53         ` Stefan Monnier
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2021-10-05 19:53 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

>> 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




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-05 19:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-05  3:01 eval-when-compile help? Dmitry Gutov
2021-10-05 12:19 ` Stefan Monnier
2021-10-05 14:16   ` Dmitry Gutov
2021-10-05 15:05     ` Stefan Monnier
2021-10-05 16:22       ` Dmitry Gutov
2021-10-05 19:53         ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).