bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Yuta Yamada]

Hello, emacs developers.

I found a weird behavior related to byte compile.

In following code named foo.el, `byte-compile-file' return t even
though there is an error message in compile log buffer.
(The `t' means the compilation was no errors, according to the function's
description.)

---- code ----
;; -*- lexical-binding: t; -*-
(defun foo ()
  (let* ((a nil))
    (add-to-list 'a "foo")))

(provide 'foo)
---- code end ----

In *Compile-Log* buffer, I got:

---- log ----
Compiling /home/uta/local/vcs/github.com/yuutayamada/emacs.d/elisp/self/hoge/foo.el...

In foo:
foo.el:6:13:Error: `add-to-list' can't use lexical var `a'; use `push' or
    `cl-pushnew'
---- log end ----

But actually `byte-compile-file' return `t'.
(I also tried "emacs -Q -batch -f batch-byte-compile ./foo.el", but
I got status code 0 after executing the command.)


I updated emacs (master branch) today.


I'm using cask for testing elisp files, which uses
`byte-recompile-file' function to build .el files, but this
byte compiler error can not detect due to the return value.


Thanks

Yuta

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Michael Heerdegen]

Yuta Yamada <sleepboy.zzz@gmail.com> writes:

> ---- code ----
> ;; -*- lexical-binding: t; -*-
> (defun foo ()
>   (let* ((a nil))
>     (add-to-list 'a "foo")))
>
> (provide 'foo)
> ---- code end ----
>
> In *Compile-Log* buffer, I got:
>
> ---- log ----
> Compiling /home/uta/local/vcs/github.com/yuutayamada/emacs.d/elisp/self/hoge/foo.el...
>
> In foo:
> foo.el:6:13:Error: `add-to-list' can't use lexical var `a'; use `push' or
>     `cl-pushnew'
> ---- log end ----
>
> But actually `byte-compile-file' return `t'.

AFAICT technically this is only a warning (displayed with
`display-warning' and level :error).  So compiling does indeed succeed I
think.

See the compiler macro defined in the defun of `add-to-list' for
internals.



Michael.

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Yuta Yamada]

> AFAICT technically this is only a warning (displayed with
> `display-warning' and level :error).  So compiling does indeed succeed I
> think.

> See the compiler macro defined in the defun of `add-to-list' for
> internals.


> Michael.

Hi, I checked add-to-list fucntion in subr.el and I saw some fixme
comments. Is this actually a warning? If so, should this warning
return non zero status code when users specified `(setq
byte-compile-error-on-warn t)`?

I tried following command to compile:

----
$ emacs -Q -batch --eval '(setq byte-compile-error-on-warn t)' -f batch-byte-compile ./foo.el

In foo:
foo.el:9:13:Error: `add-to-list' can't use lexical var `a'; use `push' or
    `cl-pushnew'
$ echo $?
0
----

But, it returns 0. Is this desired behavior? or a bug?

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Michael Heerdegen]

Yuta Yamada <sleepboy.zzz@gmail.com> writes:

> Hi, I checked add-to-list fucntion in subr.el and I saw some fixme
> comments. Is this actually a warning? If so, should this warning
> return non zero status code when users specified `(setq
> byte-compile-error-on-warn t)`?

When there is a variable of the same in the dynamic scope, the
code will work.  So, a warning seems reasonable.


> I tried following command to compile:
>
> ----
> $ emacs -Q -batch --eval '(setq byte-compile-error-on-warn t)' -f
> batch-byte-compile ./foo.el
>
> In foo:
> foo.el:9:13:Error: `add-to-list' can't use lexical var `a'; use `push' or
>     `cl-pushnew'
> $ echo $?
> 0

> But, it returns 0. Is this desired behavior? or a bug?

Seems it's wanted.  The compiler calls `byte-compile-log-warning' in
this case, which, unlike `byte-compile-warn', circumvents
`byte-compile-error-on-warn'.

Someone with more knowledge about the byte compiler might want to
correct me.


I think you could get what you want by customizing `warning-levels',
like

(setcdr (cdr (assoc :error warning-levels))
        (list (lambda () (error "Error!"))))

or so.  I'm not sure if this is a good idea.



Michael.

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Seems it's wanted.  The compiler calls `byte-compile-log-warning' in
> this case, which, unlike `byte-compile-warn', circumvents
> `byte-compile-error-on-warn'.

I wonder if we could/should call `byte-compile-warn' instead.

But Yuta, please tell me about your use case.  A lot of other warnings
printed while byte compiling can hint to similarly serious mistakes in
the code.

Would changing the behavior of this particular case really make things
better?  Can't you just have a look at the compiler warnings, which is a
good idea (necessary) anyway?


Thanks,

Michael.

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Yuta Yamada]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> > Seems it's wanted.  The compiler calls `byte-compile-log-warning' in
> > this case, which, unlike `byte-compile-warn', circumvents
> > `byte-compile-error-on-warn'.

> I wonder if we could/should call `byte-compile-warn' instead.

I found this web page (sorry if you already knew):
https://www.gnu.org/software/emacs/manual/html_node/elisp/Warning-Basics.html

And it describes:

"If you want to take the trouble to implement a way to continue
processing despite the bad data, then reporting a warning of severity
:error is the right way to inform the user of the problem. For
instance, the Emacs Lisp byte compiler can report an error that way
and continue compiling other functions. (If the program signals a Lisp
error and then handles it with condition-case, the user won ’ t see the
error message; it could show the message to the user by reporting it
as a warning.)"

So, seems there is an intent about this warning.

> But Yuta, please tell me about your use case.  A lot of other warnings
> printed while byte compiling can hint to similarly serious mistakes in
> the code.

> Would changing the behavior of this particular case really make things
> better?  Can't you just have a look at the compiler warnings, which is a
> good idea (necessary) anyway?

When I write elisp code myself, current behavior is ok. I can see the
pseudo error (or warning) message from flycheck/flymake, but when I
review other people's elisp codes on GitHub, I'm using Travis to check
byte compile warnings or errors and if tests are passed, I expects
there is no byte compile errors and I normally don't check build
result. (I think other elisp package maintainers too.)

For example, this is a build result when I noticed this error:
https://travis-ci.org/nim-lang/nim-mode/jobs/126456294

There are many noises to check errors and warnings and I'm hoping
returning non-zero status code in this case, so I can reduce the
chance to see the build result. (even this is a warning,
"Error: ..." sentence made me imagine I got an error...)


I understood that treating this warning as error hides other byte
compile warnings, so how about adding extra code to
`byte-compile-log-warning` function or make new function for this type
of warning that emits "Error: ...." message, but actually it's a warning?

----
(defun byte-compile-log-warning (string &optional fill level)
  (let ((warning-prefix-function 'byte-compile-warning-prefix)
	(warning-type-format "")
	(warning-fill-prefix (if fill "    ")))
    (display-warning 'bytecomp string level byte-compile-log-buffer)
    ;; I added below code
    ;; the `byte-compiler-error-flag' changes final result of
    ;; byte-compile-file.
    (when (or byte-compile-error-on-warn (eq level :error))
      (setq byte-compiler-error-flag t))))
----


Thanks

Yuta

bug#23495: byte-compile-file return t when there is "XXX can't use lexical var" error

[Michael Heerdegen]

Yuta Yamada <sleepboy.zzz@gmail.com> writes:

> (defun byte-compile-log-warning (string &optional fill level)
>   (let ((warning-prefix-function 'byte-compile-warning-prefix)
> 	(warning-type-format "")
> 	(warning-fill-prefix (if fill "    ")))
>     (display-warning 'bytecomp string level byte-compile-log-buffer)
>     ;; I added below code
>     ;; the `byte-compiler-error-flag' changes final result of
>     ;; byte-compile-file.
>     (when (or byte-compile-error-on-warn (eq level :error))
>       (setq byte-compiler-error-flag t))))

Dunno if something like that what would be accepted.  OTOH, if the
behavior could be controlled by a user option, it would not harm.

I leave it to somebody else to decide that.


Michael.