bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Michael Heerdegen]

Hello,

here is the test case:

#+begin_src emacs-lisp
;; -*- lexical-binding: t -*-

(require 'pcase)
(require 'find-func) ;not involved AFAICT, just funs used in example

(defun my-find-function (function)
  (interactive (find-function-read))
  (pcase (find-function-library function)
    ((or 'nil
         (and `(,_ . ,file) (guard (or (not (stringp file)) (string-match-p "\\.c$" file))))
         (guard (not (find-definition-noselect function nil))))
     (describe-function function))))
#+end_src

Compiling gives me:

| compile-test.el:6:1: Warning: Variable `file' left uninitialized
| compile-test.el:6:1: Warning: Variable `file' left uninitialized

TIA,

Michael.


In GNU Emacs 28.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-03-12 built on drachen
Repository revision: 69b952bcac5366df4cd37a7681318b29bc23e3c8
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SOUND
THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Compiling gives me:
>
> | compile-test.el:6:1: Warning: Variable `file' left uninitialized
> | compile-test.el:6:1: Warning: Variable `file' left uninitialized

This is perhaps the same problem that's causing this warning on the
current Emacs trunk?

In toplevel form:
progmodes/sh-script.el:1050:1: Warning: Variable `syntax' left uninitialized

I was just looking at this, and it looks like a spurious warning, if I
read the syntax-propertize-rules correctly.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This is perhaps the same problem that's causing this warning on the
> current Emacs trunk?
>
> In toplevel form:
> progmodes/sh-script.el:1050:1: Warning: Variable `syntax' left uninitialized

Looks indeed similar: the macro expansion (syntax-propertize-rules)
binds the variable warned about to `nil'.  The latest changes in the
byte compiler don't seem to handle this class of cases as we want.

> I was just looking at this, and it looks like a spurious warning, if I
> read the syntax-propertize-rules correctly.

I think we can wait for Stefan's reply.

Regards,

Michael.

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Stefan Monnier]

Lars Ingebrigtsen [2021-03-12 01:23:01] wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:
>> Compiling gives me:
>> | compile-test.el:6:1: Warning: Variable `file' left uninitialized
>> | compile-test.el:6:1: Warning: Variable `file' left uninitialized
>
> This is perhaps the same problem that's causing this warning on the
> current Emacs trunk?

Indeed.

> In toplevel form:
> progmodes/sh-script.el:1050:1: Warning: Variable `syntax' left uninitialized

I just fixed this one.

> I was just looking at this, and it looks like a spurious warning, if I
> read the syntax-propertize-rules correctly.

The warning complains when you initially bind a lexical var to nil
(either by not giving a value or by giving an explicit nil) and then
never set it to any other value.

Indeed, it's a useless binding: you could just use nil instead wherever
you use that var, so the warning is working as intended.  And indeed, it
found a few places where we bound a var to nil and then just
returned its value (and it's thanks to this warning that I discovered
that those var needed to be declared as dynamically scoped).

But the above `pcase` and `syntax-propertize-rules` show that it can be
quite inconvenient.  I can probably fix `pcase` to work around the
issue, but ... it's probably better to tone down the warning so it's
only issued if the nil binding is implicit rather than explicit.


        Stefan

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed, it's a useless binding: you could just use nil instead wherever
> you use that var, so the warning is working as intended.  And indeed, it
> found a few places where we bound a var to nil and then just
> returned its value (and it's thanks to this warning that I discovered
> that those var needed to be declared as dynamically scoped).

Oh, is that what the warning means.  :-)  It's not immediately
obvious -- could it be changed to something like...  er...  "Variable
bound to constant value and not changed"?

> But the above `pcase` and `syntax-propertize-rules` show that it can be
> quite inconvenient.  I can probably fix `pcase` to work around the
> issue, but ... it's probably better to tone down the warning so it's
> only issued if the nil binding is implicit rather than explicit.

Right.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Stefan Monnier]

Lars Ingebrigtsen [2021-03-12 04:07:47] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Indeed, it's a useless binding: you could just use nil instead wherever
>> you use that var, so the warning is working as intended.  And indeed, it
>> found a few places where we bound a var to nil and then just
>> returned its value (and it's thanks to this warning that I discovered
>> that those var needed to be declared as dynamically scoped).
>
> Oh, is that what the warning means.  :-)  It's not immediately
> obvious -- could it be changed to something like...  er...  "Variable
> bound to constant value and not changed"?

Indeed, the warning could even be generalized to other trivial constant
values like `t`, but I think what we have already gives enough false
positives ;-)
[ Still, I'm quite happy with the warning in that it pointed me to a few
  actual problems in the code.  The average usefulness rating of each
  of the warnings that I fixed after adding this warning was not too bad.  ]

As for changing the wording, feel free: I'm not too happy with the
current one either (after all, the variables are initialized to nil in
any case, so there's no such thing as an uninitialized variable),
tho I'm not very fond of the one you suggest.

In any case I installed the patch below which should fix the immediate offenders.


        Stefan


diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el
index ca641a2ef0..cfb0168a6e 100644
--- a/lisp/emacs-lisp/cconv.el
+++ b/lisp/emacs-lisp/cconv.el
@@ -602,7 +602,8 @@ cconv--analyze-use
      (byte-compile-warn
       "%s `%S' not left unused" varkind var))
     ((and (let (or 'let* 'let) (car form))
-          `(,(or `(,var) `(,var nil)) t nil ,_ ,_))
+          `(,`(,var) ;; (or `(,var nil) : Too many false positives: bug#47080
+            t nil ,_ ,_))
      ;; FIXME: Convert this warning to use `macroexp--warn-wrap'
      ;; so as to give better position information.
      (unless (not (intern-soft var))

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed, the warning could even be generalized to other trivial constant
> values like `t`, but I think what we have already gives enough false
> positives ;-)

FWIW, I also got one or two true positives in my own stuff.

To avoid false positives, is it possible to limit the warning to "human
written" code or to code that is not the result of macro expansion?


Thanks,

Michael.

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Stefan Monnier]

>> Indeed, the warning could even be generalized to other trivial constant
>> values like `t`, but I think what we have already gives enough false
>> positives ;-)
> FWIW, I also got one or two true positives in my own stuff.
> To avoid false positives, is it possible to limit the warning to "human
> written" code or to code that is not the result of macro expansion?

It's somewhere between difficult and impossible with the current setup.
Hygienic macro systems should be able to get that information, but
our macro system doesn't keep track of it.

It would require efforts similar to those needed to preserve
file+line+col information.


        Stefan

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It would require efforts similar to those needed to preserve
> file+line+col information.

I had hoped we could maybe benefit from that: if the variable has
line+col information, it must have been human written.  But it's
probably not reliable enough.

Michael.

bug#47080: 28.0.50; Spurious variable left uninitialized compiler warning

[Stefan Monnier]

>> It would require efforts similar to those needed to preserve
>> file+line+col information.
> I had hoped we could maybe benefit from that: if the variable has
> line+col information, it must have been human written.  But it's
> probably not reliable enough.

It's not just "not reliable enough": there's simply no variable that has
line+col info currently.


        Stefan