Understanding macroexp.el

[Michael Heerdegen]

Hello,

yesterday I tried to understand the helpers in macroexp.el.  I have a
question:

Would it make sense to install the following:

*** /tmp/ediff139941AX  2015-11-18 11:12:07.788012282 +0100
--- /home/micha/software/emacs/lisp/emacs-lisp/macroexp.el      2015-11-18 
10:47:41.041352909 +0100
***************
*** 331,337 ****
    (cond
     ((null bindings) exp)
     ((eq 'let* (car-safe exp)) `(let* (,@bindings ,@(cadr exp)) ,@(cddr exp)))
!    (t `(let* ,bindings ,exp))))
  
  (defun macroexp-if (test then else)
    "Return an expression equivalent to `(if ,TEST ,THEN ,ELSE)."
--- 331,337 ----
    (cond
     ((null bindings) exp)
     ((eq 'let* (car-safe exp)) `(let* (,@bindings ,@(cadr exp)) ,@(cddr exp)))
!    (t `(let* ,bindings ,@(macroexp-unprogn exp)))))
  
  (defun macroexp-if (test then else)
    "Return an expression equivalent to `(if ,TEST ,THEN ,ELSE)."

Here is an example where it matters:

(defmacro test (exp)
  (macroexp-let2 nil res exp
    `(progn
       (f ,res)
       (g ,res))))

(note that the `progn' is mandatory here.)  Without the patch:

(macroexpand '(test (+ 1 2)))
==>
(let*
    ((#:res #1=(+ 1 2)))
  (progn
    (f #1#)
    (g #1#)))


After patch:

(macroexpand '(test (+ 1 2)))
==>
(let*
    ((#:res #1=(+ 1 2)))
  (f #1#)
  (g #1#))


(And is it currently allowed to push this to master?)


Thanks,

Michael.