bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode

[Pip Cet <pipcet@gmail.com>]

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

Hello Andrea,

by the way, I'm mostly still pursuing it because it keeps flushing out
bugs in comp.el :-) I'm attaching another one, though I'm not sure the
bug in question can actually be triggered without modifying the byte
compiler.

On Tue, Feb 23, 2021 at 10:55 PM Andrea Corallo <akrl@sdf.org> wrote:
> Pip Cet <pipcet@gmail.com> writes:
> >> >> > We don't have to, let's just not emit an assume about a variable that
> >> > we just introduced and that's never read?
> >>
> >> We disagree :)
> >
> > We can disagree about the "let's" part (and should, of course), but we
> > should agree about the "we don't have to" part, because that's a
> > matter of fact, not opinion.
>
> This new mvar *is* used, actually by an assume.

I said it was never read, which I agree is more correct.

> And the assume in
> discussion is targetting a live variable that will be used by functional
> code so I really see no scandal here.

Only on one side of the assume. The other side holds a new "temporary"
variable which is assumed to be equal to something that it isn't
actually equal to, and that seems very scandalous to me.

> >> As the byte compiler does not care about propagating types and values it
> >> can just clobber the variable, here we aim for more so we need it to
> >> keep it live till the assume.
> >
> > If we decide the variable needs to be kept live, the byte compiler
> > should keep it live, not us.
>
> Again no, any pass in the compiler must have the right to create new
> temporaries for whichever purpose it wants, and indeed we can't expect

I agree with that part, by the way. We should, at some point in the
future when we need it, add the ability to introduce temporary
variables (and remove them), in a clean, well-considered fashion. At
present, we don't need it, we lack the ability to remove such
variables, and I'm afraid the last parts can be disputed, too.

[-- Attachment #2: 0001-Fix-ICE-when-compiling-an-explicit-call-to-single-ar.patch --]
[-- Type: text/x-patch, Size: 944 bytes --]

From f75e874d50a2e22a9ff9df2fcbfbf5e74d6bbff5 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Wed, 24 Feb 2021 06:58:24 +0000
Subject: [PATCH] Fix ICE when compiling an explicit call to single-argument
 `-'

* lisp/emacs-lisp/comp.el (comp-fwprop-call): Handle
negation as well as subtraction.
---
 lisp/emacs-lisp/comp.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/comp.el b/lisp/emacs-lisp/comp.el
index 60c040926e54c..2980605bf8804 100644
--- a/lisp/emacs-lisp/comp.el
+++ b/lisp/emacs-lisp/comp.el
@@ -3040,7 +3040,7 @@ comp-fwprop-call
               (comp-mvar-neg lval) (comp-cstr-neg cstr))))
     (cl-case f
       (+ (comp-cstr-add lval args))
-      (- (comp-cstr-sub lval args))
+      (- (if (cdr args) (comp-cstr-sub lval args)))
       (1+ (comp-cstr-add lval `(,(car args) ,comp-cstr-one)))
       (1- (comp-cstr-sub lval `(,(car args) ,comp-cstr-one))))))
 
-- 
2.30.0