bug#28254: 26.0.50; SRFI-2 and-let*

[Mark Oteiza]

On 13/09/17 at 12:16pm, Michael Heerdegen wrote:
> Mark Oteiza <mvoteiza@udel.edu> writes:
> 
> > > In `internal--listify':
> > > isn't (or (listp form) (atom form)) always true?
> >
> > Yes, that could instead be (or form (null form)). It's meant to catch
> > things like this:
> >
> >   (should (equal nil (and-let* ((nil) (x 1)))))
> 
> Why can't we just replace it with the equivalent `t' (and simplify the
> code accordingly)?

Oh goodness… thanks.

> > > Secondly, in `internal--build-binding-value-form':
> > > How can it happen that (car binding) is an atom but not symbolp?  And if
> > > (car binding) == var is not a symbol, how does the returned binding make
> > > sense?
> >
> > It's an expression, like a number.
> >
> >   (should (equal 1 (and-let* ((2) (x 1)))))
> 
> AFAICT, this doesn't run the code I mention.  If I trace
> `internal--build-binding-value-form' and try this, I get
> 
> 1 -> (internal--build-binding-value-form (#:s nil) t)
> 1 <- internal--build-binding-value-form: (#:s (and t nil))
> ======================================================================
> 1 -> (internal--build-binding-value-form (x 1) #:s)
> 1 <- internal--build-binding-value-form: (x (and #:s 1))
> 
> In both cases, (car binding) evals to a symbol.

I guess the following is fine then--thanks for finding these.

diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el
index 3ea01065c8..ba0ab6cb4c 100644
--- a/lisp/emacs-lisp/subr-x.el
+++ b/lisp/emacs-lisp/subr-x.el
@@ -87,9 +87,7 @@ internal--listify
 If ELT is of the form ((EXPR)), listify (EXPR) with a dummy symbol."
   (cond
    ((symbolp elt) (list elt elt))
-   ((and (null (cdr elt))
-         (let ((form (car elt)))
-           (or (listp form) (atom form))))
+   ((null (cdr elt))
     (list (make-symbol "s") (car elt)))
    (t elt)))
 
@@ -104,9 +102,7 @@ internal--check-binding
 (defsubst internal--build-binding-value-form (binding prev-var)
   "Build the conditional value form for BINDING using PREV-VAR."
   (let ((var (car binding)))
-    (if (and (null (cdr binding)) (atom (car binding)) (not (symbolp (car 
binding))))
-        `(,var (and ,prev-var ,var))
-      `(,var (and ,prev-var ,(cadr binding))))))
+    `(,var (and ,prev-var ,(cadr binding)))))
 
 (defun internal--build-binding (binding prev-var)
   "Check and build a single BINDING with PREV-VAR."