bug#41065: Result of byte-compilation can be nil even if the compiled file contains no errors

bug#41065: Result of byte-compilation can be nil even if the compiled file contains no errors

[Paul Pogonyshev]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

To reproduce, save this as file `main.el':

    (eval-when-compile
      (let ((default-directory (expand-file-name "/tmp/myprivatelib/")))
        (make-directory default-directory t)
        (with-temp-file "thelib.el"
          (insert "(defun im-a-library-function ()\n"
                  "  (trigger-a-warning-here-1))\n"
                  "(provide 'thelib\n"))  ; <- intentionally erroneous
syntax
        (byte-compile-file "thelib.el")))

    (defun blabla ()
      (trigger-a-warning-here-2))

and execute:

    $ emacs --batch --eval "(print (byte-compile-file \"main.el\"))"

Result is:

    In toplevel form:
    thelib.el:3:1: Error: End of file during parsing

    In end of data:
    ~/test/nested-compilation/main.el:12:1: Warning: the function
        ‘trigger-a-warning-here-2’ is not known to be defined.

    nil

The last line is the result of the call to `byte-compile-file'
from the command line and it is nil.  But this is wrong, because
`main.el' is syntactically correct and never triggers an error:
you can verify this by evaluating `let' form manually.  What
failed is the "inner" compilation, but it must not affect the
result of the "main" compilation process.

A smaller problem is that the path to `main.el' is written fully,
because it is apparently found from `/tmp/myprivatelib', which
further corroborates my guess that the two byte-compilation
processes are improperly lumped together.

If you fix generated `thelib.el' by adding the missing paren,
"outer" compilation will succeed.

I emailed this to to `emacs-devel' quite a while ago:

    https://lists.gnu.org/archive/html/emacs-devel/2020-02/msg00227.html

and the answer from Stefan Monnier confirmed that this should be
qualified as a bug.  I have forgotten to resubmit this to
`bug-gnu-emacs' until now.

Paul

[-- Attachment #2: Type: text/html, Size: 2308 bytes --]

bug#41065: PATCH

[Paul Pogonyshev]

[-- Attachment #1.1: Type: text/plain, Size: 24 bytes --]

Trivial patch attached.

[-- Attachment #1.2: Type: text/html, Size: 45 bytes --]

[-- Attachment #2: 0001-Don-t-leak-result-of-nested-byte-compilation-to-oute.patch --]
[-- Type: text/x-patch, Size: 1299 bytes --]

From 87d927a3a61f16fa908b5dd1c00457023f269114 Mon Sep 17 00:00:00 2001
From: Paul Pogonyshev <pogonyshev@gmail.com>
Date: Tue, 27 Oct 2020 01:08:19 +0100
Subject: [PATCH] Don't leak result of nested byte-compilation to outer level
 (bug#41065)

---
 lisp/emacs-lisp/bytecomp.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index a547b672b1..cbda16d051 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1938,7 +1938,7 @@ byte-compile-file
         (byte-compile--known-dynamic-vars
          (byte-compile--load-dynvars (getenv "EMACS_DYNVARS_FILE")))
 	target-file input-buffer output-buffer
-	byte-compile-dest-file)
+	byte-compile-dest-file byte-compiler-error-flag)
     (setq target-file (byte-compile-dest-file filename))
     (setq byte-compile-dest-file target-file)
     (with-current-buffer
@@ -2000,7 +2000,6 @@ byte-compile-file
 	  'no-byte-compile)
       (when byte-compile-verbose
 	(message "Compiling %s..." filename))
-      (setq byte-compiler-error-flag nil)
       ;; It is important that input-buffer not be current at this call,
       ;; so that the value of point set in input-buffer
       ;; within byte-compile-from-buffer lingers in that buffer.
-- 
2.20.1

bug#41065: (no subject)

[Lars Ingebrigtsen]

Paul Pogonyshev <pogonyshev@gmail.com> writes:

> Subject: [PATCH] Don't leak result of nested byte-compilation to outer level
>  (bug#41065)

[...]

> +	byte-compile-dest-file byte-compiler-error-flag)
>      (setq target-file (byte-compile-dest-file filename))
>      (setq byte-compile-dest-file target-file)
>      (with-current-buffer
> @@ -2000,7 +2000,6 @@ byte-compile-file
>  	  'no-byte-compile)
>        (when byte-compile-verbose
>  	(message "Compiling %s..." filename))
> -      (setq byte-compiler-error-flag nil)

Thanks; applied to Emacs 28.  I tried to follow the code paths where
this variable is used, and as far as I can tell, this should be safe.

The old code was rather odd, because there's just a

(defvar byte-compiler-error-flag)

in the file -- it's not actually defined, but after compiling something,
that `setq' would define it globally.  So your patch fixes that problem,
too.

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