Re: master f9d7440: ; * lisp/emacs-lisp/byte-opt.el (byte-optimize-eq): Fix last change.

Re: master f9d7440: ; * lisp/emacs-lisp/byte-opt.el (byte-optimize-eq): Fix last change.

[Pip Cet]

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

On Tue, Jul 20, 2021 at 5:32 PM Mattias Engdegård
<mattiase@savannah.gnu.org> wrote:
> diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
> index 7ed04b3..c9c0ac0 100644
> --- a/lisp/emacs-lisp/byte-opt.el
> +++ b/lisp/emacs-lisp/byte-opt.el
> @@ -970,10 +970,9 @@ See Info node `(elisp) Integer Basics'."
>       form)))
>
>  (defun byte-optimize-eq (form)
> -  (byte-optimize-binary-predicate
> -   (pcase (cdr form)
> -     ((or `(,x nil) `(nil ,x)) `(not ,x))
> -     (_ form))))
> +  (pcase (cdr form)
> +    ((or `(,x nil) `(nil ,x)) `(not ,x))
> +    (_ (byte-optimize-binary-predicate form))))

I played around with simple expressions a little after this patch, and
things didn't seem quite right. The good news is they weren't quite
right without the patch, either.

I find things work better with the attached patch applied. If it is as
obviously correct as it seems to me, it should probably be applied.

But it does occur to me this code optimizes (quote) to nil. Is that
intentional? Is it worth fixing? (It also optimizes (quote 1 2 3) to
1).

Pip

[-- Attachment #2: 0001-Fix-test-in-byte-optimize-quote.patch --]
[-- Type: text/x-diff, Size: 816 bytes --]

From e31764707462ba278549aace71b699ef04127cd2 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Tue, 20 Jul 2021 19:58:16 +0000
Subject: [PATCH] Fix test in byte-optimize-quote

*  (byte-optimize-quote): Fix condition not to be trivially false.
---
 lisp/emacs-lisp/byte-opt.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index c9c0ac0045..39ec5aa9fd 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -1077,7 +1077,7 @@ byte-optimize-concat
 (defun byte-optimize-quote (form)
   (if (or (consp (nth 1 form))
 	  (and (symbolp (nth 1 form))
-	       (not (macroexp--const-symbol-p form))))
+	       (not (macroexp--const-symbol-p (nth 1 form)))))
       form
     (nth 1 form)))
 
-- 
2.32.0

Re: master f9d7440: ; * lisp/emacs-lisp/byte-opt.el (byte-optimize-eq): Fix last change.

[Mattias Engdegård]

20 juli 2021 kl. 22.07 skrev Pip Cet <pipcet@gmail.com>:

> I played around with simple expressions a little after this patch, and
> things didn't seem quite right.

Oh no...

> The good news is they weren't quite
> right without the patch, either.

Phew! Got away this time.

> I find things work better with the attached patch applied. If it is as
> obviously correct as it seems to me, it should probably be applied.

Good spotting! I don't see why not. In fact it probably allows us to remove my ad-hoc 'nil => nil at the end of `byte-optimize-form`.

> But it does occur to me this code optimizes (quote) to nil. Is that
> intentional? Is it worth fixing? (It also optimizes (quote 1 2 3) to
> 1).

At least (quote 1 2 3) gives a warning -- I'd say it should be a hard error but the compiler seems to be soft on law and order, damn hippies -- but (quote) doesn't even reach `byte-optimize-quote`; it's converted to nil in `byte-optimize-form-code-walker` without as much as a squeak.

Re: master f9d7440: ; * lisp/emacs-lisp/byte-opt.el (byte-optimize-eq): Fix last change.

[Mattias Engdegård]

>> In fact it probably allows us to remove my ad-hoc 'nil => nil at the end of `byte-optimize-form`.

Now done, along with your proposed fix, and made so that (quote) at least elicits a warning.
Thank you!