GNU bug report logs - #79910
31.0.50; 8815194ea6d: Byte-compiled closures incorrectly merged due to overly aggressive constant equality check

Previous Next

To reply to this bug, email your comments to 79910 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Yue Yi" <include_yy <at> qq.com>
To: "andrei.elkin--- via Bug reports for GNU Emacs,the Swiss army knife of text editors"
 <bug-gnu-emacs <at> gnu.org>
Subject: 31.0.50;
 8815194ea6d: Byte-compiled closures incorrectly merged due to overly
 aggressive constant equality check
Date: Sun, 30 Nov 2025 00:27:50 +0800

Hello, Emacs maintainers,

I am reporting a byte-compiler regression introduced by Commit
8815194ea6d (Unify constants that are equal-including-properties in
compiler). This change leads to structurally similar lexical closures
being incorrectly treated as equal constants and being merged (shared)
during compilation.

--------------------------->8<-------------------------------------------
@@ -3870,8 +3870,7 @@ byte-compile-variable-set
       (byte-compile-dynamic-variable-op 'byte-varset var))))
 
 (defmacro byte-compile-get-constant (const)
-  `(or (assoc ,const byte-compile-constants
-              (if (stringp ,const) #'equal-including-properties #'eql))
+  `(or (assoc ,const byte-compile-constants #'equal-including-properties)
        (car (setq byte-compile-constants
 		  (cons (list ,const) byte-compile-constants)))))
--------------------------->8<-------------------------------------------

As the commit message stated, this change drastically reduces the size
of .elc files by eliminating redundant constants.  However, compared to
merely merging numeric, symbolic, or string content, this also leads to
the merging of the byte-compiled results for closures that are
structurally similar. For example, consider the following illustration
shown to me by my friend:

--------------------------->8<-------------------------------------------
;; -*- lexical-binding: t; -*-

(defmacro once! (&rest body)
  (let ((evaluatedp nil) (value nil))
    (let ((evalp (lambda () evaluatedp))
          (give-value (lambda () value))
          (store-value
           (lambda (x)
             (setq evaluatedp t)
             (setq value x))))
      `(if (funcall ,evalp)
           (funcall ,give-value)
         (funcall ,store-value (progn ,@body))))))

(defun fun (x) (once! (eval x)))

(defun test () (cl-loop for i from 0 to 10 collect (fun i)))
--------------------------->8<-------------------------------------------

This once! macro achieves a thunk-like effect by inserting closures
evaluated during macro expansion. After byte-compiling it with
byte-compile-file and loading it with load-file, executing (test) yields
the output (0 t t t t t t t t t t) instead of all zeros. This occurs
because the two lambda closures within the first compiled once! macro
are found to be equal in their byte-compiled function form. During the
compilation of fun, they are recognized as equal and subsequently merged
by the modified byte-compile-get-constant function.

Of course, it is well known that compiler-computed products should
generally be treated as "constants" to avoid unexpected issues, and this
is best practice in most cases, unless we know exactly what we are
doing. From an optimization perspective, I believe this commit is fine,
but from my friend's point of view, he argues that functions are
inherently incomparable, and perhaps we should consider avoiding
treating functions as comparable objects.

It is simple to work around the problem demonstrated in the example
above, either by changing the initial value of evaluatedp to another
non-nil value, or by rewriting the code logic to use only one function.

--------------------------->8<-------------------------------------------
(defmacro once! (&rest body)
  (let (ev va)
    (let ((thunk (lambda (f) (if ev va (setq ev t va (funcall f))))))
      `(funcall ,thunk (lambda () ,@body)))))
--------------------------->8<-------------------------------------------

WDYT?

Regards.

Message #8 received at 79910 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "Yue Yi" <include_yy <at> qq.com>,
 Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 79910 <at> debbugs.gnu.org
Subject: Re: bug#79910: 31.0.50;
 8815194ea6d: Byte-compiled closures incorrectly merged due to overly
 aggressive constant equality check
Date: Sat, 29 Nov 2025 18:57:09 +0200

> Date: Sun, 30 Nov 2025 00:27:50 +0800
> From:  "Yue Yi" via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Hello, Emacs maintainers,
> 
> I am reporting a byte-compiler regression introduced by Commit
> 8815194ea6d (Unify constants that are equal-including-properties in
> compiler). This change leads to structurally similar lexical closures
> being incorrectly treated as equal constants and being merged (shared)
> during compilation.
> 
> --------------------------->8<-------------------------------------------
> @@ -3870,8 +3870,7 @@ byte-compile-variable-set
>        (byte-compile-dynamic-variable-op 'byte-varset var))))
>  
>  (defmacro byte-compile-get-constant (const)
> -  `(or (assoc ,const byte-compile-constants
> -              (if (stringp ,const) #'equal-including-properties #'eql))
> +  `(or (assoc ,const byte-compile-constants #'equal-including-properties)
>         (car (setq byte-compile-constants
>  		  (cons (list ,const) byte-compile-constants)))))
> --------------------------->8<-------------------------------------------
> 
> As the commit message stated, this change drastically reduces the size
> of .elc files by eliminating redundant constants.  However, compared to
> merely merging numeric, symbolic, or string content, this also leads to
> the merging of the byte-compiled results for closures that are
> structurally similar. For example, consider the following illustration
> shown to me by my friend:
> 
> --------------------------->8<-------------------------------------------
> ;; -*- lexical-binding: t; -*-
> 
> (defmacro once! (&rest body)
>   (let ((evaluatedp nil) (value nil))
>     (let ((evalp (lambda () evaluatedp))
>           (give-value (lambda () value))
>           (store-value
>            (lambda (x)
>              (setq evaluatedp t)
>              (setq value x))))
>       `(if (funcall ,evalp)
>            (funcall ,give-value)
>          (funcall ,store-value (progn ,@body))))))
> 
> (defun fun (x) (once! (eval x)))
> 
> (defun test () (cl-loop for i from 0 to 10 collect (fun i)))
> --------------------------->8<-------------------------------------------
> 
> This once! macro achieves a thunk-like effect by inserting closures
> evaluated during macro expansion. After byte-compiling it with
> byte-compile-file and loading it with load-file, executing (test) yields
> the output (0 t t t t t t t t t t) instead of all zeros. This occurs
> because the two lambda closures within the first compiled once! macro
> are found to be equal in their byte-compiled function form. During the
> compilation of fun, they are recognized as equal and subsequently merged
> by the modified byte-compile-get-constant function.
> 
> Of course, it is well known that compiler-computed products should
> generally be treated as "constants" to avoid unexpected issues, and this
> is best practice in most cases, unless we know exactly what we are
> doing. From an optimization perspective, I believe this commit is fine,
> but from my friend's point of view, he argues that functions are
> inherently incomparable, and perhaps we should consider avoiding
> treating functions as comparable objects.
> 
> It is simple to work around the problem demonstrated in the example
> above, either by changing the initial value of evaluatedp to another
> non-nil value, or by rewriting the code logic to use only one function.
> 
> --------------------------->8<-------------------------------------------
> (defmacro once! (&rest body)
>   (let (ev va)
>     (let ((thunk (lambda (f) (if ev va (setq ev t va (funcall f))))))
>       `(funcall ,thunk (lambda () ,@body)))))
> --------------------------->8<-------------------------------------------
> 
> WDYT?

Thanks, I've added Mattias and Stefan to the discussion.

Message #11 received at 79910 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Mattias Engdegård <mattias.engdegard <at> gmail.com>,
 Yue Yi <include_yy <at> qq.com>, 79910 <at> debbugs.gnu.org
Subject: Re: bug#79910: 31.0.50; 8815194ea6d: Byte-compiled closures
 incorrectly merged due to overly aggressive constant equality check
Date: Sat, 29 Nov 2025 13:18:40 -0500

>> (defmacro once! (&rest body)
>>   (let ((evaluatedp nil) (value nil))
>>     (let ((evalp (lambda () evaluatedp))
>>           (give-value (lambda () value))
>>           (store-value
>>            (lambda (x)
>>              (setq evaluatedp t)
>>              (setq value x))))
>>       `(if (funcall ,evalp)
>>            (funcall ,give-value)
>>          (funcall ,store-value (progn ,@body))))))

He the 3 closures are included in the output code as literal constants
(you didn't write a `quote` in front of the comma, but if they weren't
self-quoting (e.g. in older Emacsen) that could cause an error).

Literal constants should be immutable, whereas yours are mutated (from
within, but that's the same problem).  For that reason this code is incorrect.

>> Of course, it is well known that compiler-computed products should
>> generally be treated as "constants" to avoid unexpected issues, and
>> this is best practice in most cases, unless we know exactly what we
>> are doing. From an optimization perspective, I believe this commit is
>> fine, but from my friend's point of view, he argues that functions
>> are inherently incomparable, and perhaps we should consider avoiding
>> treating functions as comparable objects.

I don't see this as being qualitatively any different from the usual
mutation of quoted constants.

Side note: including function values (i.e. values that contain already
runnable code) from compile-time into the output code of the macro,
i.e. into source code, like the above macro does, is problematic in
a few ways.

The fundamental issue is that the function value is supposed to be
opaque, yet the compiler needs to look inside of it.  It doesn't really
have a well-defined and stable representation and the compiler can't
know if the intention is to keep this value *as-is* or if it's OK to
rewrite it as long as it behaves identically.  E.g. it could have been
compiled into a #<subr...>, which can't be saved into a `.elc` file.
Or on the contrary it could be an "interpreted closure" and will be kept
as-is in the output, thus will fail to be byte-compiled.


        Stefan

Message #14 received at 79910 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Yue Yi <include_yy <at> qq.com>,
 79910 <at> debbugs.gnu.org
Subject: Re: bug#79910: 31.0.50; 8815194ea6d: Byte-compiled closures
 incorrectly merged due to overly aggressive constant equality check
Date: Sat, 29 Nov 2025 21:28:06 +0100

29 nov. 2025 kl. 19.18 skrev Stefan Monnier <monnier <at> iro.umontreal.ca>:

> Side note: including function values (i.e. values that contain already
> runnable code) from compile-time into the output code of the macro,
> i.e. into source code, like the above macro does, is problematic in
> a few ways.

Indeed. The print syntax for function values isn't really part of the source syntax, even when it's read syntax.

Message #17 received at 79910 <at> debbugs.gnu.org (full text, mbox):

From: "Yue Yi" <include_yy <at> qq.com>
To: "Mattias Engdegård" <mattias.engdegard <at> gmail.com>,
 "Stefan Monnier" <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 79910 <79910 <at> debbugs.gnu.org>
Subject: Re: bug#79910: 31.0.50;
 8815194ea6d: Byte-compiled closures incorrectly merged due to
 overlyaggressive constant equality check
Date: Sun, 30 Nov 2025 12:46:49 +0800

Stefan Monnier wrote,

> Literal constants should be immutable, whereas yours are mutated (from
> within, but that's the same problem).  For that reason this code is incorrect.
>
> [...]
>
> The fundamental issue is that the function value is supposed to be
> opaque, yet the compiler needs to look inside of it.  It doesn't really
> have a well-defined and stable representation and the compiler can't
> know if the intention is to keep this value *as-is* or if it's OK to
> rewrite it as long as it behaves identically.

I understand and agree that literal constants should be immutable. By
inserting closures that capture mutable state into the macro expansion
result and subsequently modifying that state at runtime, the code
operates outside of safe boundaries.

Mattias Engdegård wrote,

> 29 nov. 2025 kl. 19.18 skrev Stefan Monnier <monnier <at> iro.umontreal.ca>:

> > Side note: including function values (i.e. values that contain already
> > runnable code) from compile-time into the output code of the macro,
> > i.e. into source code, like the above macro does, is problematic in
> > a few ways.

> Indeed. The print syntax for function values isn't really part of the source 
> syntax, even when it's read syntax.

Thanks for explaination, and feel free to close this bug report :)

Message #20 received at 79910 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: 79910 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, include_yy <at> qq.com
Subject: Re: bug#79910: 31.0.50; 8815194ea6d: Byte-compiled closures
 incorrectly merged due to overly aggressive constant equality check
Date: Sun, 30 Nov 2025 07:41:26 +0200

> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Sat, 29 Nov 2025 21:28:06 +0100
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>  Yue Yi <include_yy <at> qq.com>,
>  79910 <at> debbugs.gnu.org
> 
> 29 nov. 2025 kl. 19.18 skrev Stefan Monnier <monnier <at> iro.umontreal.ca>:
> 
> > Side note: including function values (i.e. values that contain already
> > runnable code) from compile-time into the output code of the macro,
> > i.e. into source code, like the above macro does, is problematic in
> > a few ways.
> 
> Indeed. The print syntax for function values isn't really part of the source syntax, even when it's read syntax.

The ELisp manual describes both the print syntax and the read syntax
of the important objects.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.