Re: tex.el compiler warnings

[Arash Esbati]

Tassilo Horn <tsdh@gnu.org> writes:

> Arash Esbati <arash@gnu.org> writes:
>
>> 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.

I'm not an expert on this, but the lexical binding version of it would
be:

    (defun TeX-foobar ()
      (let (_)
        (defvar file)
        (let ((file something))
          (TeX-quuz)
          (TeX-baz))))

Right?  If so, then I suggest I really don't hold my breath for a fix
and we go directly towards lexical binding .  I did a test on tex.el by
removing the `with-no-warnings', added -*- lexical-binding: t -*- to it
and byte-compiled the file.  The log is:

    In toplevel form:
    tex.el:1188:1: Warning: Unused lexical variable `dbus-debug'

    In TeX-pdf-tools-sync-view:
    tex.el:1240:24: Warning: reference to free variable `file'

    In TeX-evince-sync-view-1:
    tex.el:1264:43: Warning: reference to free variable `file'
    tex.el:2015:1: Warning: Unused lexical argument `viewer'
    tex.el:3123:1: Warning: Unused lexical variable `upcase'

    In TeX-parse-argument:
    tex.el:3532:26: Warning: reference to free variable `exit-mark'

    In TeX-argument-insert:
    tex.el:3556:13: Warning: assignment to free variable 
`last-optional-rejected'
    tex.el:3561:37: Warning: reference to free variable `exit-mark'
    tex.el:3608:1: Warning: Unused lexical argument `optional'
    tex.el:3950:1: Warning: Unused lexical variable `name'
    tex.el:3970:1: Warning: Unused lexical variable `name'
    tex.el:4280:1: Warning: Unused lexical argument `a'
    tex.el:4992:1: Warning: Unused lexical variable `file'
    tex.el:5184:1: Warning: Unused lexical argument `pos'
    tex.el:5282:1: Warning: Unused lexical variable `i'

    In TeX-math-input-method-off:
    tex.el:5979:11: Warning: `inactivate-input-method' is an obsolete function 
(as
        of 24.3); use `deactivate-input-method' instead.
    tex.el:6179:1: Warning: Unused lexical argument `button'
    tex.el:6179:1: Warning: Unused lexical argument `button'
    tex.el:6390:1: Warning: Unused lexical variable `doc'
    tex.el:6452:7: Warning: assignment to free variable 
`ispell-enable-tex-parser'
    tex.el:6454:1: Warning: Unused lexical argument `string'
    tex.el:6454:1: Warning: Unused lexical argument `command'
    tex.el:6468:1: Warning: Unused lexical variable `found'

A transition looks feasible to me.

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

I agree,in general.  OTOH, the warnings were there for quite a long
period and the code wasn't touched -- my assumption was we've got used
to it :-)

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

Fine with me; I'll revert the `with-no-warnings'.

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

I think this is clear.

Best, Arash