Re: tex.el compiler warnings

[Tassilo Horn]

Arash Esbati <arash@gnu.org> writes:

Hi Arash,

>> Why is that needed?  As far as I see, that function only complains
>> about variables defined by AUCTeX itself which you can also defvar,
>> no?
>
> Yes, the complaints about free variables `TeX-*' vanish by defvar'ing
> them.

That's good.

>> In TeX-pdf-tools-sync-view:
>> tex.el:1187:11: Warning: reference to free variable ‘TeX-PDF-mode’
>> tex.el:1191:12: Warning: reference to free variable
>>     ‘TeX-source-correlate-mode’
>> tex.el:1193:38: Warning: reference to free variable
>>     ‘TeX-current-process-region-p’
>> tex.el:1197:24: Warning: reference to free variable ‘file’
>>
>> Of course with the exception of the variable `file' set via dynamic
>> scoping which is one hindrance for switching to lexical-binding and
>> as such should produce a compiler warning.
>
> Yep, `file' is the only warning which remains, defvar'ing doesn't help
> here (lack of prefix warning)

Yes, don't defvar it.

>> Maybe we should add a
>>
>>   (defvar TeX-file nil)
>>
>> and bind that in addition to the undeclared `file', make AUCTeX
>> itself only use TeX-file (and update the docs), and eventually omit
>> binding `file' in the mid-term future.
>
> That would be fine with me, I can commit the defvar's only and omit
> the `with-no-warnings' if you would cater for the rest :-)

Yes, please.  We only want to silence warnings which don't indicate real
issues.  All "declared in another file which will be loaded at runtime
anyway" belong in that category.  The one with `file' is a real issue.
It means we do

  (defun TeX-foobar ()
    (let ((file something))
      (TeX-quux)
      (TeX-baz)))

and rely on TeX-quux and TeX-baz (and everything called from there)
having access to the local variable `file'.  That works with dynamic
binding but is ugly and prevents us from switching to lexical binding.
So what we have to do is change that to

  (devar TeX-file nil)
  (defun TeX-foobar ()
    (let ((TeX-file something))
      (TeX-quux)
      (TeX-baz)))

and make TeX-quux and TeX-baz (and everything called below) use
`TeX-file' instead of `file'.  The byte-compiler warnings hint us
towards the usages and that's why they are valuable.  (Additionally, if
user's have their own commands accessing `file', they need to switch to
TeX-file, too.)

I'll try to fix that but don't hold your breath.

>> Same here.
>>
>> In TeX-evince-sync-view-1:
>> tex.el:1221:43: Warning: reference to free variable ‘file’
>> tex.el:1230:40: Warning: reference to free variable
>>     ‘TeX-current-process-region-p’
>>
>> Ah, or is it complaining about the dbus functions?  I guess they are
>> only available on GNU/Linux (and you didn't turn off dbus support).
>
> My understanding here was `file' again and not dbus.
>
>> But anyway, I would like to keep warnings about abusing dynamic scope
>> such as the warning about `file' here.
>
> I'd really like to see AUCTeX building clean.

There's absolutely no value in having a build with no warnings if that
inhibits warnings indicating real issues.

> Can we make a compromise?  If you can get rid of `file' complaints by
> changing the code, that would be great.  Otherwise, we use
> `with-no-warnings' and add I'll add some comments to tex.el (and
> latex.el and context.el) for functions where it should be removed
> first once we move towards lexical-binding.

The first option, please!  Compiler warnings are much better to be
looked up/navigated and always reflect the actual state whereas comments
might be missed easily.

So please keep all warnings about references to free, unprefixed
variables.

Ah, and I think we also have to distinguish two cases with references to
free _prefixed_ variables.  If the byte-compiler warns about a reference
to a free variable TeX-something, we have to check:

  1. If it is declared as (defvar TeX-something nil), i.e., with a value
     in some other file, put a (defvar TeX-something) in the current
     file as you have done.

  2. If it is *not* declared anywhere, i.e., there's no such defvar
     anywhere or just a (defvar TeX-something) without a value, then it
     needs to be declared somewhere (with value)!

The reason for point 2 is that (defvar TeX-something) is just a a hint
to the byte-compiler that TeX-something is declared somewhere else.  It
is no declaration, i.e., it won't declare that variable as special
(dynamically scoped).

Bye,
Tassilo