bug#58801: [PATCH] Autoload the `calc-eval-error' variable

[Matt Armstrong <matt@rfc20.org>]

Stefan Kangas <stefankangas@gmail.com> writes:

> Matt Armstrong <matt@rfc20.org> writes:
>
>> From 526d0b31e0d836e7a3c21d831849b8c50da2420e Mon Sep 17 00:00:00 2001
>> From: Matt Armstrong <matt@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.