bug#54990: Byte compiler bug

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello, Emacs.

In up to date master branch:
(i) emacs -Q
(ii) Enter the following into buffer *scratch*:

    (defun E+->E@+ (elt op)
      (cond
       ((eq op '+) (setcar elt '*))
       ((eq op '+\?) (setcar elt '*\?))))
    (defconst foo-elt '(+\? . "foo"))
    (E+->E@+ foo-elt '+\?)

  .
(iii) With point in the defun, M-x compile-defun.
(iv) Evaluate the defconst.
(v) Evaluate the E+->E@+ form.
(vi) M-: foo-elt.  It's value is unchanged from its declaration.  It
  should have been changed to (*\? . "foo").  This is a bug.

(vii) M-x disassemble RET E+->E@+.  Instead of working, this gives the
  error message:

    Optimizer error: missed tags (((TAG 2) TAG 4) ((TAG 1) TAG 3))

  .  This is a bug.

(viii) (Optional)  Evaluate the defun form with C-M-x, and evaluate the
  E+->E@+ form.  M-: foo-elt.  This shows the expected value,
  (*? . "foo").

At a guess, the symbols with the \? in their names have something to do
with the bug.

Here is my configuration:

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.31, cairo version 1.16.0)
 of 2022-04-17 built on ACM
Repository revision: 2136db067f4292d84553ebfddab30d88b862262e
Repository branch: master
System Description: Gentoo/Linux

Configured using:
 'configure --with-gif=no --with-tiff=no --with-gpm
 --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XPM
GTK3 ZLIB

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#54990: Byte compiler bug

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> At a guess, the symbols with the \? in their names have something to do
> with the bug.

Could this be related to 637dde4ab?

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

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello, Lars.

On Sun, Apr 17, 2022 at 15:02:33 +0200, Lars Ingebrigtsen wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > At a guess, the symbols with the \? in their names have something to do
> > with the bug.

> Could this be related to 637dde4ab?

It isn't, no.  The bug is in git checkout 637dde4ab^.

I've checked Emacs 28.1, the bug is not there, thankfully.

It looks like I've got an afternoon of bisecting facing me.  ;-)

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

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#54990: Byte compiler bug

[Mattias Engdegård]

This is related to symbol positioning so Alan should be well-placed to debug it. (The symbol names are irrelevant.)

It's `compile-defun` that is broken; the constant vector of the resulting bytecode contains symbols with position as hash table keys for a switch operation. Compare the bytecode object with that from `byte-compile` which works correctly:

(defun tata (x)
  (cond
   ((eq x 'a) 'toto)
   ((eq x 'b) 'titi)))

(byte-compile 'tata)
=>
#[257 "..." [#s(hash-table size 2 test eq rehash-size 1.5 rehash-threshold 0.8125 purecopy t data (a 6 b 8)) toto titi nil] 3 "..."]

;; if using `compile-defun` on `tata`:
(symbol-function 'tata)
#[257 "..." [#s(hash-table size 2 test eq rehash-size 1.5 rehash-threshold 0.8125 purecopy t data (#<symbol a at 293> 6 #<symbol b at 314> 8)) toto titi nil] 3 "..."]

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello, Mattias.

On Sun, Apr 17, 2022 at 16:32:53 +0200, Mattias Engdegård wrote:
> This is related to symbol positioning so Alan should be well-placed to
> debug it. (The symbol names are irrelevant.)

I'd got just as far as identifying the merge of
scratch/correct-warning-pos as the first version containing the bug.

> It's `compile-defun` that is broken; the constant vector of the
> resulting bytecode contains symbols with position as hash table keys
> for a switch operation. Compare the bytecode object with that from
> `byte-compile` which works correctly:

Thank you for this observation.  It was _exceptionally_ helpful.

> (defun tata (x)
>   (cond
>    ((eq x 'a) 'toto)
>    ((eq x 'b) 'titi)))

> (byte-compile 'tata)
> =>
> #[257 "..." [#s(hash-table size 2 test eq rehash-size 1.5 rehash-threshold 0.8125 purecopy t data (a 6 b 8)) toto titi nil] 3 "..."]

> ;; if using `compile-defun` on `tata`:
> (symbol-function 'tata)
> #[257 "..." [#s(hash-table size 2 test eq rehash-size 1.5 rehash-threshold 0.8125 purecopy t data (#<symbol a at 293> 6 #<symbol b at 314> 8)) toto titi nil] 3 "..."]

I've committed a fix, which I'm pretty sure works, so I'm closing the bug
with this post.  Thanks again for the help!

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#54990: Byte compiler bug

[Mattias Engdegård]

18 apr. 2022 kl. 12.48 skrev Alan Mackenzie <acm@muc.de>:

> I've committed a fix, which I'm pretty sure works, so I'm closing the bug
> with this post.

Very happy to see that you found a solution quickly!

A few items of concern:

1. There should be a regression test for it -- don't you agree?

2. Your solution comprises stripping position from symbols during the lowering of lapcode to bytecode. Why did this bug only affect `compile-defun`, not `byte-compile` or `byte-compile-file`? The change is in code common to all three.

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello, Mattias.

On Mon, Apr 18, 2022 at 17:43:46 +0200, Mattias Engdegård wrote:
> 18 apr. 2022 kl. 12.48 skrev Alan Mackenzie <acm@muc.de>:

> > I've committed a fix, which I'm pretty sure works, so I'm closing the bug
> > with this post.

> Very happy to see that you found a solution quickly!

> A few items of concern:

> 1. There should be a regression test for it -- don't you agree?

I suppose so.

> 2. Your solution comprises stripping position from symbols during the
> lowering of lapcode to bytecode.

Yes.  It preserves symbols' positions as long as possible.

> Why did this bug only affect `compile-defun`, not `byte-compile` or
> `byte-compile-file`? The change is in code common to all three.

Yes, I find that intriguing, too.  I'm meaning to try to find out why
that was the case.  The first priority was to fix the bug.  The
differences between the ways the compiler was called might hide further
bugs, though.

So, I haven't forgotten this.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello again, Mattias.

On Wed, Apr 20, 2022 at 19:33:41 +0000, Alan Mackenzie wrote:
> On Mon, Apr 18, 2022 at 17:43:46 +0200, Mattias Engdegård wrote:
> > 18 apr. 2022 kl. 12.48 skrev Alan Mackenzie <acm@muc.de>:

[ .... ]

> > Why did this bug only affect `compile-defun`, not `byte-compile` or
> > `byte-compile-file`? The change is in code common to all three.

> Yes, I find that intriguing, too.  I'm meaning to try to find out why
> that was the case.  The first priority was to fix the bug.  The
> differences between the ways the compiler was called might hide further
> bugs, though.

This is now clear.  byte-compile is compiling a form, not source code,
so unless the form was read with symbol positions, it won't have them.
In byte-compile-file, symbol positions are stripped as the byte code
gets output to the file.elc.  So the symbol positions were not in the
..elc.

Only in compile-defun were there positions, and they "survived".

So I don't think there is a problem here.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#54990: Byte compiler bug

[Mattias Engdegård]

21 apr. 2022 kl. 11.49 skrev Alan Mackenzie <acm@muc.de>:

> This is now clear.  byte-compile is compiling a form, not source code,
> so unless the form was read with symbol positions, it won't have them.
> In byte-compile-file, symbol positions are stripped as the byte code
> gets output to the file.elc.  So the symbol positions were not in the
> ..elc.
> 
> Only in compile-defun were there positions, and they "survived".

Thank you for dealing with this. My next question would be why the bug only affected switch hash table keys and no other constants. Do you know?

And why did you decide to strip the keys at that point (lowering from LAP), and not where other constants are stripped?

I see that you have committed more corrections since; were these related?

bug#54990: Byte compiler bug

[Alan Mackenzie]

Hello, Mattias.

On Mon, May 02, 2022 at 15:45:25 +0200, Mattias Engdegård wrote:
> 21 apr. 2022 kl. 11.49 skrev Alan Mackenzie <acm@muc.de>:

> > This is now clear.  byte-compile is compiling a form, not source code,
> > so unless the form was read with symbol positions, it won't have them.
> > In byte-compile-file, symbol positions are stripped as the byte code
> > gets output to the file.elc.  So the symbol positions were not in the
> > ..elc.

> > Only in compile-defun were there positions, and they "survived".

> Thank you for dealing with this. My next question would be why the bug
> only affected switch hash table keys and no other constants. Do you
> know?

The handling of a cond form which compiles to a hash table is anamolous.
This table is essentially byte code, yet is created in an early phase of
the compilation, when symbols still have their positions.  So these
positions have to be handled specially.  Other forms are converted to
byte code in a later phase of the compiler, when (nearly) all symbols
have their positions stripped.

> And why did you decide to strip the keys at that point (lowering from
> LAP), and not where other constants are stripped?

To be honest, I can't really remember; I might not have been aware of any
anomaly, here.  As a general principle, the positions on symbols are
preserved as long as possible, in case a warning message is yet to be
output.

> I see that you have committed more corrections since; were these
> related?

No, they were not.  They were to do with other problems.

-- 
Alan Mackenzie (Nuremberg, Germany).