Minor simplification in byte-opt.el

Minor simplification in byte-opt.el

[Stefan Monnier]

Could someone look over the patch below to confirm it's safe?

Also, as a result of this, there is only one call to
`byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
the two functions could be merged, tho it's probably not worth the trouble.


        Stefan


    * lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker):
    After inlining, use `byte-optimize-form` so the top-level function's
    optimizer (if any) is also applied.
    Simplify the pure function handling by using `byte-optimize-constant-args`.
    (byte-optimize-all-constp): Remove.
    (byte-optimize-1+, byte-optimize-1-): Remove, since `pure` already
    takes care of it.


diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 6f801be545..afe07da0ec 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -227,7 +227,7 @@ byte-compile-log-lap
 \f
 ;;; byte-compile optimizers to support inlining
 
-(put 'inline 'byte-optimizer 'byte-optimize-inline-handler)
+(put 'inline 'byte-optimizer #'byte-optimize-inline-handler)
 
 (defun byte-optimize-inline-handler (form)
   "byte-optimize-handler for the `inline' special-form."
@@ -396,7 +396,7 @@ byte-optimize-form-code-walker
 	     (if (eq newform form)
 		 ;; Some error occurred, avoid infinite recursion
 		 form
-	       (byte-optimize-form-code-walker newform for-effect))))
+	       (byte-optimize-form newform for-effect))))
 	  ((eq (car-safe fn) 'closure) form)
 	  ((memq fn '(let let*))
 	   ;; recursively enter the optimizer for the bindings and body
@@ -554,23 +554,10 @@ byte-optimize-form-code-walker
 	   ;; Otherwise, no args can be considered to be for-effect,
 	   ;; even if the called function is for-effect, because we
 	   ;; don't know anything about that function.
-	   (let ((args (mapcar #'byte-optimize-form (cdr form))))
-	     (if (and (get fn 'pure)
-		      (byte-optimize-all-constp args))
-                 (let ((arg-values (mapcar #'eval args)))
-                   (condition-case nil
-                       (list 'quote (apply fn arg-values))
-                     (error (cons fn args))))
-	       (cons fn args)))))))
-
-(defun byte-optimize-all-constp (list)
-  "Non-nil if all elements of LIST satisfy `macroexp-const-p'."
-  (let ((constant t))
-    (while (and list constant)
-      (unless (macroexp-const-p (car list))
-	(setq constant nil))
-      (setq list (cdr list)))
-    constant))
+	   (let ((form (cons fn (mapcar #'byte-optimize-form (cdr form)))))
+	     (if (get fn 'pure)
+		 (byte-optimize-constant-args form)
+	       form))))))
 
 (defun byte-optimize-form (form &optional for-effect)
   "The source-level pass of the optimizer."
@@ -747,22 +734,6 @@ byte-optimize-minus
        ((equal args (cdr form)) form)
        (t (cons '- args))))))
 
-(defun byte-optimize-1+ (form)
-  (let ((args (cdr form)))
-    (when (null (cdr args))
-      (let ((n (car args)))
-        (when (numberp n)
-          (setq form (1+ n))))))
-  form)
-
-(defun byte-optimize-1- (form)
-  (let ((args (cdr form)))
-    (when (null (cdr args))
-      (let ((n (car args)))
-        (when (numberp n)
-          (setq form (1- n))))))
-  form)
-
 (defun byte-optimize-multiply (form)
   (let* ((args (remq 1 (byte-opt--arith-reduce #'* 1 (cdr form)))))
     (cond
@@ -802,7 +773,7 @@ byte-optimize-binary-predicate
     (condition-case ()
         (list 'quote (eval form))
       (error form)))
-   (t ;; This can enable some lapcode optimizations.
+   (t ;; Moving the constant to the end can enable some lapcode optimizations.
     (list (car form) (nth 2 form) (nth 1 form)))))
 
 (defun byte-optimize-constant-args (form)
@@ -901,37 +872,34 @@ byte-optimize-concat
         form          ; No improvement.
       (cons 'concat (nreverse newargs)))))
 
-(put 'identity 'byte-optimizer 'byte-optimize-identity)
-(put 'memq 'byte-optimizer 'byte-optimize-memq)
-(put 'memql  'byte-optimizer 'byte-optimize-member)
-(put 'member 'byte-optimizer 'byte-optimize-member)
-(put 'assoc 'byte-optimizer 'byte-optimize-assoc)
-(put 'rassoc 'byte-optimizer 'byte-optimize-assoc)
-
-(put '+   'byte-optimizer 'byte-optimize-plus)
-(put '*   'byte-optimizer 'byte-optimize-multiply)
-(put '-   'byte-optimizer 'byte-optimize-minus)
-(put '/   'byte-optimizer 'byte-optimize-divide)
-(put 'max 'byte-optimizer 'byte-optimize-associative-math)
-(put 'min 'byte-optimizer 'byte-optimize-associative-math)
-
-(put '=   'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'eq  'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'eql   'byte-optimizer 'byte-optimize-equal)
-(put 'equal 'byte-optimizer 'byte-optimize-equal)
-(put 'string= 'byte-optimizer 'byte-optimize-binary-predicate)
-(put 'string-equal 'byte-optimizer 'byte-optimize-binary-predicate)
-
-(put '1+  'byte-optimizer 'byte-optimize-1+)
-(put '1-  'byte-optimizer 'byte-optimize-1-)
-
-(put 'concat 'byte-optimizer 'byte-optimize-concat)
+(put 'identity 'byte-optimizer #'byte-optimize-identity)
+(put 'memq 'byte-optimizer #'byte-optimize-memq)
+(put 'memql  'byte-optimizer #'byte-optimize-member)
+(put 'member 'byte-optimizer #'byte-optimize-member)
+(put 'assoc 'byte-optimizer #'byte-optimize-assoc)
+(put 'rassoc 'byte-optimizer #'byte-optimize-assoc)
+
+(put '+   'byte-optimizer #'byte-optimize-plus)
+(put '*   'byte-optimizer #'byte-optimize-multiply)
+(put '-   'byte-optimizer #'byte-optimize-minus)
+(put '/   'byte-optimizer #'byte-optimize-divide)
+(put 'max 'byte-optimizer #'byte-optimize-associative-math)
+(put 'min 'byte-optimizer #'byte-optimize-associative-math)
+
+(put '=   'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'eq  'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'eql   'byte-optimizer #'byte-optimize-equal)
+(put 'equal 'byte-optimizer #'byte-optimize-equal)
+(put 'string= 'byte-optimizer #'byte-optimize-binary-predicate)
+(put 'string-equal 'byte-optimizer #'byte-optimize-binary-predicate)
+
+(put 'concat 'byte-optimizer #'byte-optimize-concat)
 
 ;; I'm not convinced that this is necessary.  Doesn't the optimizer loop
 ;; take care of this? - Jamie
 ;; I think this may some times be necessary to reduce ie (quote 5) to 5,
 ;; so arithmetic optimizers recognize the numeric constant.  - Hallvard
-(put 'quote 'byte-optimizer 'byte-optimize-quote)
+(put 'quote 'byte-optimizer #'byte-optimize-quote)
 (defun byte-optimize-quote (form)
   (if (or (consp (nth 1 form))
 	  (and (symbolp (nth 1 form))
@@ -1049,16 +1017,16 @@ byte-optimize-while
   (if (nth 1 form)
       form))
 
-(put 'and   'byte-optimizer 'byte-optimize-and)
-(put 'or    'byte-optimizer 'byte-optimize-or)
-(put 'cond  'byte-optimizer 'byte-optimize-cond)
-(put 'if    'byte-optimizer 'byte-optimize-if)
-(put 'while 'byte-optimizer 'byte-optimize-while)
+(put 'and   'byte-optimizer #'byte-optimize-and)
+(put 'or    'byte-optimizer #'byte-optimize-or)
+(put 'cond  'byte-optimizer #'byte-optimize-cond)
+(put 'if    'byte-optimizer #'byte-optimize-if)
+(put 'while 'byte-optimizer #'byte-optimize-while)
 
 ;; byte-compile-negation-optimizer lives in bytecomp.el
-(put '/= 'byte-optimizer 'byte-compile-negation-optimizer)
-(put 'atom 'byte-optimizer 'byte-compile-negation-optimizer)
-(put 'nlistp 'byte-optimizer 'byte-compile-negation-optimizer)
+(put '/= 'byte-optimizer #'byte-compile-negation-optimizer)
+(put 'atom 'byte-optimizer #'byte-compile-negation-optimizer)
+(put 'nlistp 'byte-optimizer #'byte-compile-negation-optimizer)
 
 
 (defun byte-optimize-funcall (form)
@@ -1086,12 +1054,12 @@ byte-optimize-apply
 	      nil))
 	form)))
 
-(put 'funcall 'byte-optimizer 'byte-optimize-funcall)
-(put 'apply   'byte-optimizer 'byte-optimize-apply)
+(put 'funcall 'byte-optimizer #'byte-optimize-funcall)
+(put 'apply   'byte-optimizer #'byte-optimize-apply)
 
 
-(put 'let 'byte-optimizer 'byte-optimize-letX)
-(put 'let* 'byte-optimizer 'byte-optimize-letX)
+(put 'let 'byte-optimizer #'byte-optimize-letX)
+(put 'let* 'byte-optimizer #'byte-optimize-letX)
 (defun byte-optimize-letX (form)
   (cond ((null (nth 1 form))
 	 ;; No bindings
@@ -1107,7 +1075,7 @@ byte-optimize-letX
 	   (list 'let* (reverse (cdr binds)) (nth 1 (car binds)) nil)))))
 
 
-(put 'nth 'byte-optimizer 'byte-optimize-nth)
+(put 'nth 'byte-optimizer #'byte-optimize-nth)
 (defun byte-optimize-nth (form)
   (if (= (safe-length form) 3)
       (if (memq (nth 1 form) '(0 1))
@@ -1117,7 +1085,7 @@ byte-optimize-nth
 	form)
     form))
 
-(put 'nthcdr 'byte-optimizer 'byte-optimize-nthcdr)
+(put 'nthcdr 'byte-optimizer #'byte-optimize-nthcdr)
 (defun byte-optimize-nthcdr (form)
   (if (= (safe-length form) 3)
       (if (memq (nth 1 form) '(0 1 2))
@@ -1133,7 +1101,7 @@ byte-optimize-nthcdr
 ;; optimize string-as-unibyte, string-as-multibyte, string-make-unibyte,
 ;; string-make-multibyte for constant args.
 
-(put 'set 'byte-optimizer 'byte-optimize-set)
+(put 'set 'byte-optimizer #'byte-optimize-set)
 (defun byte-optimize-set (form)
   (let ((var (car-safe (cdr-safe form))))
     (cond

Re: Minor simplification in byte-opt.el

[Mattias Engdegård]

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

26 juli 2020 kl. 22.41 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Could someone look over the patch below to confirm it's safe?

Thank you, looks decent to me. Good that you spotted the 1+/1-, I forgot about those.

> Also, as a result of this, there is only one call to
> `byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
> the two functions could be merged, tho it's probably not worth the trouble.

Maybe -- there seems to be little to gain from integrating them other than the elimination of the function call.

By the way, what about this little improvement?


[-- Attachment #2: 0001-Simplify-and-streamline-optimizer-clauses.patch --]
[-- Type: application/octet-stream, Size: 2507 bytes --]

From cc6cbb304193c62354d3cb6b0b76f8ce066b2189 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mattias=20Engdeg=C3=A5rd?= <mattiase@acm.org>
Date: Mon, 27 Jul 2020 11:27:00 +0200
Subject: [PATCH] Simplify and streamline optimizer clauses

* lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker):
Remove clause for 'with-output-to-temp-buffer', since it is a
macro and will have been expanded before reaching this point.
Move clauses for 'lambda' and 'closure' to avoid splitting
a cond jump table.
---
 lisp/emacs-lisp/byte-opt.el | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 6f801be545..48efff911f 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -391,13 +391,6 @@ byte-optimize-form-code-walker
 	   (and (nth 1 form)
 		(not for-effect)
 		form))
-	  ((eq (car-safe fn) 'lambda)
-	   (let ((newform (byte-compile-unfold-lambda form)))
-	     (if (eq newform form)
-		 ;; Some error occurred, avoid infinite recursion
-		 form
-	       (byte-optimize-form-code-walker newform for-effect))))
-	  ((eq (car-safe fn) 'closure) form)
 	  ((memq fn '(let let*))
 	   ;; recursively enter the optimizer for the bindings and body
 	   ;; of a let or let*.  This for depth-firstness: forms that
@@ -444,13 +437,6 @@ byte-optimize-form-code-walker
 	   ;; will be optimized away in the lap-optimize pass.
 	   (cons fn (byte-optimize-body (cdr form) for-effect)))
 
-	  ((eq fn 'with-output-to-temp-buffer)
-	   ;; this is just like the above, except for the first argument.
-	   (cons fn
-	     (cons
-	      (byte-optimize-form (nth 1 form) nil)
-	      (byte-optimize-body (cdr (cdr form)) for-effect))))
-
 	  ((eq fn 'if)
 	   (when (< (length form) 3)
 	     (byte-compile-warn "too few arguments for `if'"))
@@ -530,6 +516,15 @@ byte-optimize-form-code-walker
           ;; Needed as long as we run byte-optimize-form after cconv.
           ((eq fn 'internal-make-closure) form)
 
+	  ((eq (car-safe fn) 'lambda)
+	   (let ((newform (byte-compile-unfold-lambda form)))
+	     (if (eq newform form)
+		 ;; Some error occurred, avoid infinite recursion
+		 form
+	       (byte-optimize-form-code-walker newform for-effect))))
+
+	  ((eq (car-safe fn) 'closure) form)
+
           ((byte-code-function-p fn)
            (cons fn (mapcar #'byte-optimize-form (cdr form))))
 
-- 
2.21.1 (Apple Git-122.3)

Re: Minor simplification in byte-opt.el

[Stefan Monnier]

>> Could someone look over the patch below to confirm it's safe?
> Thank you, looks decent to me. Good that you spotted the 1+/1-, I forgot about those.

Thanks.

>> Also, as a result of this, there is only one call to
>> `byte-optimize-form-code-walker` left, in `byte-optimize-form`, so maybe
>> the two functions could be merged, tho it's probably not worth the trouble.
> Maybe -- there seems to be little to gain from integrating them other than
> the elimination of the function call.

Exactly :-(

> By the way, what about this little improvement?

Looks good [ tho IMO we shouldn't support `closure` there.  ]


        Stefan

Re: Minor simplification in byte-opt.el

[Mattias Engdegård]

27 juli 2020 kl. 16.24 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> [ tho IMO we shouldn't support `closure` there.  ]

Probably. I was a bit unsure about whether that case could ever occur and didn't want to change too many things at once.

Re: Minor simplification in byte-opt.el

[Stefan Monnier]

>> [ tho IMO we shouldn't support `closure` there.  ]
> Probably. I was a bit unsure about whether that case could ever occur and
> didn't want to change too many things at once.

We currently do allow it (in the interpreter and the byte-compiler), but
I think this is a mistake and we should emit a warning (if not an error)
when the byte-compiler encounters such a construct.


        Stefan

Re: Minor simplification in byte-opt.el

[Mattias Engdegård]

28 juli 2020 kl. 16.53 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> We currently do allow it (in the interpreter and the byte-compiler), but
> I think this is a mistake and we should emit a warning (if not an error)
> when the byte-compiler encounters such a construct.

Yes, sorry, I meant that I wasn't sure whether could be synthesised (by cconv) without the user actually ever feeding an explicit 'closure' construct into the compiler. After all, 'closure' isn't really in the language, is it?

Re: Minor simplification in byte-opt.el

[Stefan Monnier]

>> We currently do allow it (in the interpreter and the byte-compiler), but
>> I think this is a mistake and we should emit a warning (if not an error)
>> when the byte-compiler encounters such a construct.
>
> Yes, sorry, I meant that I wasn't sure whether could be synthesised (by
> cconv) without the user actually ever feeding an explicit 'closure'
> construct into the compiler. After all, 'closure' isn't really in the
> language, is it?

No, indeed, it's not in the language.  The way it usually shows up is
when a macro stuffs a *value* in there, e.g.

    `(,(get foo 'frobbing-function) ...)

The more correct alternative

    `(funcall ',(get foo 'frobbing-function) ...)

tends to suffer from false-positive warnings about quoted lambdas when
the `frobbing-function` was not byte-compiled and comes from a file that
still doesn't use lexical-binding.



        Stefan