GNU bug report logs - #58801
[PATCH] Autoload the `calc-eval-error' variable

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58801 in the body.
You can then email your comments to 58801 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Autoload the `calc-eval-error' variable
Date: Wed, 26 Oct 2022 10:02:50 -0700

[Message part 1 (text/plain, inline)]

Tags: patch

(rationale in the patch)

In GNU Emacs 29.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.16.0) of 2022-10-25 built on naz
Repository revision: 9d7ba2b1998afc3664c37d9d1b6f6ca2d68356e9
Repository branch: feature/noverlay
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure 'CFLAGS=-Og -g3' 'CXXFLAGS=-Og -g3' --enable-checking=yes
 --enable-check-lisp-object-type --with-pgtk'

[0001-Autoload-the-calc-eval-error-variable.patch (text/patch, attachment)]

Message #8 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 58801 <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Fri, 11 Nov 2022 05:16:42 -0800

Matt Armstrong <matt <at> rfc20.org> writes:

> From 526d0b31e0d836e7a3c21d831849b8c50da2420e Mon Sep 17 00:00:00 2001
> From: Matt Armstrong <matt <at> rfc20.org>
> Date: Wed, 26 Oct 2022 09:46:37 -0700
> Subject: [PATCH] Autoload the `calc-eval-error' variable
>
> * lisp/calc/calc-aent.el: Autoload the `calc-eval-error' variable,
> because it is documented as a lisp level option of the `calc-eval'
> function, which is also autoloaded.  Otherwise, even (require 'calc)
> is not enough to get the variable defined; `calc-eval' must actually
> be evaluated.  This squashes byte compiler warnings in code using the
> variable.

I don't necessarily object strongly or anything, but should we really
autoload a variable just to squash byte compiler warnings?

I think the usual way to do that is to say

    (defvar calc-eval-error)

in the calling code.

> ---
>  lisp/calc/calc-aent.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/calc/calc-aent.el b/lisp/calc/calc-aent.el
> index ef3e0d4b67..59692beff7 100644
> --- a/lisp/calc/calc-aent.el
> +++ b/lisp/calc/calc-aent.el
> @@ -252,6 +252,7 @@ calc-do-calc-eval
>  			     res (cdr res)))
>  		     buf)))))))))
>
> +;;;###autoload
>  (defvar calc-eval-error nil
>    "Determines how calc handles errors.
>  If nil, return a list containing the character position of error.

Message #13 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 58801 <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Tue, 15 Nov 2022 10:24:00 -0800

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Matt Armstrong <matt <at> rfc20.org> writes:
>
>> From 526d0b31e0d836e7a3c21d831849b8c50da2420e Mon Sep 17 00:00:00 2001
>> From: Matt Armstrong <matt <at> rfc20.org>
>> Date: Wed, 26 Oct 2022 09:46:37 -0700
>> Subject: [PATCH] Autoload the `calc-eval-error' variable
>>
>> * lisp/calc/calc-aent.el: Autoload the `calc-eval-error' variable,
>> because it is documented as a lisp level option of the `calc-eval'
>> function, which is also autoloaded.  Otherwise, even (require 'calc)
>> is not enough to get the variable defined; `calc-eval' must actually
>> be evaluated.  This squashes byte compiler warnings in code using the
>> variable.
>
> I don't necessarily object strongly or anything, but should we really
> autoload a variable just to squash byte compiler warnings?

Perhaps I can learn something here.  Why refrain from autoloading the
variable in this situation?

Note that in my case I had (require 'calc) in the file that used the
`calc-eval-error' symbol.  The info docs for calc state that (require
'calc) loads nearly everything you need from calc.  I may not understand
something about the design constraints here, but it seems strange to
refrain from autoloading this symbol, since (require 'calc) already
(auto)loads a *lot* of stuff.

> I think the usual way to do that is to say
>
>     (defvar calc-eval-error)
>
> in the calling code.

I think "in the calling code" applies to specific situations.  For
example:

 - A defvar for something x- in package x.

 - Symbols provided by packages that are conditionally loaded, so the
   current package can not rely on (require 'x) to providing `x-'
   symbols at bytcomp time.

 - Situations where the package has inadequate/incorrect autoloads, so
   (require 'x) doesn't provide enough.  I.e. to work around bugs.  ;-)


My first impression is that adding `defvar' to squash bytecomp warnings
for symbols in other packages is the wrong default action, and that the
best idea is for

  (require 'foo)

to provide all symbols 'foo-' that one might need when using the `foo'
package in the normal way.

Notice that (info "(elisp) Converting to Lexical Binding") has this
phrasing:

> A warning about a reference or an assignment to a free variable is
> usually a clear sign that that variable should be marked as
> dynamically scoped, so you need to add an appropriate ‘defvar’ before
> the first use of that variable.

It doesn't state what an "appropriate 'defvar'" is.  Certainly, if the
var is part of the current package, adding a 'defvar' in the same file
makes sense.  If the var is part of some other package, properly
required by the current package, I think that other package is missing
an autoload.

Message #16 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 58801 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Tue, 15 Nov 2022 20:42:08 +0200

> Cc: 58801 <at> debbugs.gnu.org
> From: Matt Armstrong <matt <at> rfc20.org>
> Date: Tue, 15 Nov 2022 10:24:00 -0800
> 
> My first impression is that adding `defvar' to squash bytecomp warnings
> for symbols in other packages is the wrong default action

No, it's the standard solution for byte-compilation warnings.  There
are a few others, all of them better than an actual require.

> and that the best idea is for
> 
>   (require 'foo)
> 
> to provide all symbols 'foo-' that one might need when using the `foo'
> package in the normal way.

That actually loads the package foo, for no good reason.  We don't
necessarily want to use foo, we just want to compile a reference to
its variable or function.

Message #19 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 58801 <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Thu, 24 Nov 2022 11:50:56 -0800

Matt Armstrong <matt <at> rfc20.org> writes:

> Note that in my case I had (require 'calc) in the file that used the
> `calc-eval-error' symbol.  The info docs for calc state that (require
> 'calc) loads nearly everything you need from calc.  I may not understand
> something about the design constraints here, but it seems strange to
> refrain from autoloading this symbol, since (require 'calc) already
> (auto)loads a *lot* of stuff.

So you are saying that if you have a file foo.el, that requires calc,
and then tries to use calc-eval-error variable (documented as part of
the external API), you get a byte-compiler warning?

I agree that this doesn't sound very intuitive.

> My first impression is that adding `defvar' to squash bytecomp warnings
> for symbols in other packages is the wrong default action, and that the
> best idea is for
>
>   (require 'foo)
>
> to provide all symbols 'foo-' that one might need when using the `foo'
> package in the normal way.

So I think we could install your patch.

Message #22 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 58801 <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Sat, 26 Nov 2022 08:58:53 -0800

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Matt Armstrong <matt <at> rfc20.org> writes:
>
>> Note that in my case I had (require 'calc) in the file that used the
>> `calc-eval-error' symbol.  The info docs for calc state that (require
>> 'calc) loads nearly everything you need from calc.  I may not understand
>> something about the design constraints here, but it seems strange to
>> refrain from autoloading this symbol, since (require 'calc) already
>> (auto)loads a *lot* of stuff.
>
> So you are saying that if you have a file foo.el, that requires calc,
> and then tries to use calc-eval-error variable (documented as part of
> the external API), you get a byte-compiler warning?
>
> I agree that this doesn't sound very intuitive.

I regret typing about `require' at all, as my line of argument is
simpler than that.

Running "emacs -Q" comes with `calc-eval' autoloaded.  Since calc
documentation mentions `calc-eval-error' as a configuration variable for
the `calc-eval' behavior, it is makes most sense to autoload either
neither of them or both of them.

(In the particular case of the Calc package, dozens of functions and
variables are already autoloaded.  The omission of `calc-eval-error'
also seems more an oversight than intentional.)


> So I think we could install your patch.

Me too.  ;-)

Message #25 received at 58801 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 58801 <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Thu, 7 Sep 2023 00:51:57 -0700

Matt Armstrong <matt <at> rfc20.org> writes:

> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Matt Armstrong <matt <at> rfc20.org> writes:
>>
>>> Note that in my case I had (require 'calc) in the file that used the
>>> `calc-eval-error' symbol.  The info docs for calc state that (require
>>> 'calc) loads nearly everything you need from calc.  I may not understand
>>> something about the design constraints here, but it seems strange to
>>> refrain from autoloading this symbol, since (require 'calc) already
>>> (auto)loads a *lot* of stuff.
>>
>> So you are saying that if you have a file foo.el, that requires calc,
>> and then tries to use calc-eval-error variable (documented as part of
>> the external API), you get a byte-compiler warning?
>>
>> I agree that this doesn't sound very intuitive.
>
> I regret typing about `require' at all, as my line of argument is
> simpler than that.
>
> Running "emacs -Q" comes with `calc-eval' autoloaded.  Since calc
> documentation mentions `calc-eval-error' as a configuration variable for
> the `calc-eval' behavior, it is makes most sense to autoload either
> neither of them or both of them.

Thanks, now I understand the situation better.

We typically avoid autoloading variables, and I'm not sure it's
justified here.  See (info "(elisp) When to Autoload") for details.

Could we instead just declare it in calc.el?  I believe that should
silence any warnings from the byte-compiler.  It's a one line change:

    (defvar calc-eval-error)

Or will that not work in your use case for some reason?

> (In the particular case of the Calc package, dozens of functions and
> variables are already autoloaded.  The omission of `calc-eval-error'
> also seems more an oversight than intentional.)

FWIW, I couldn't find any autoloaded variables in calc-loaddefs.el.
What am I missing?

Message #30 received at 58801-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: 58801-done <at> debbugs.gnu.org
Subject: Re: bug#58801: [PATCH] Autoload the `calc-eval-error' variable
Date: Sun, 5 Nov 2023 08:38:21 -0800

Version: 30.1

Stefan Kangas <stefankangas <at> gmail.com> writes:

> We typically avoid autoloading variables, and I'm not sure it's
> justified here.  See (info "(elisp) When to Autoload") for details.
>
> Could we instead just declare it in calc.el?  I believe that should
> silence any warnings from the byte-compiler.  It's a one line change:
>
>     (defvar calc-eval-error)
>
> Or will that not work in your use case for some reason?
>
>> (In the particular case of the Calc package, dozens of functions and
>> variables are already autoloaded.  The omission of `calc-eval-error'
>> also seems more an oversight than intentional.)
>
> FWIW, I couldn't find any autoloaded variables in calc-loaddefs.el.
> What am I missing?

No further comments here within 2 months, so I've pushed the above
proposed fix to master, and I'm closing this bug.

If this is still an issue, please reply to this email (use "Reply to
all" in your email client) and we can reopen the bug report.

[1: ad82bc9b29e]: 2023-11-05 17:36:21 +0100
  Declare calc-eval-error in calc.el
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ad82bc9b29eacad29a441bbb4e87bd09ef1ff1c4

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.