* bug#28254: 26.0.50; SRFI-2 and-let* @ 2017-08-27 20:11 Mark Oteiza 2017-09-01 2:55 ` npostavs 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-08-27 20:11 UTC (permalink / raw) To: 28254 Hi, A while ago I aliased and-let* to when-let* in subr-x in be10c00d. Some time later I implemented it and promptly forgot about it. While this could be cleaned up and dropped into subr-x, it could just as well be in its own file--its behavior overlaps with when-let* but each has things the other lacks. https://www.gnu.org/software/guile/manual/html_node/SRFI_002d2.html https://srfi.schemers.org/srfi-2/srfi-2.html (defmacro and-let* (varlist &rest body) "Bind variables according to VARLIST and conditionally eval BODY. Like `when-let*' except if BODY is empty and all the bindings are non-nil, then the result is t. Elements of VARLIST can additionally be of the form (EXPR), which is evaluated and checked for nil." (declare (indent 1) (debug ([&or (&rest &or symbolp atom (symbolp &optional form) (form)) (symbolp form)] body))) (let (res (prev-var t) (i 0)) (dolist (binding varlist) (push (cond ((symbolp binding) (prog1 `(,binding (and ,prev-var ,binding)) (setq prev-var binding))) ((atom binding) binding) ((and (listp binding) (null (cdr binding)) (let ((form (car binding))) (or (listp form) (atom form)))) (let ((new (cl-gensym))) (prog1 `(,new (and ,prev-var ,(car binding))) (setq prev-var new)))) (t (prog1 `(,(car binding) (and ,prev-var ,(cadr binding))) (setq prev-var (car binding))))) res)) `(let* ,(nreverse res) ,(if (null body) prev-var `(when ,prev-var ,@body))))) (ert-deftest srfi-2-test-empty-varlist () (should (equal 1 (and-let* () 1))) (should (equal 2 (and-let* () 1 2))) (should (equal t (and-let* ())))) (ert-deftest srfi-2-test-group-1 () (should (equal nil (let ((x nil)) (and-let* (x))))) (should (equal 1 (let ((x 1)) (and-let* (x))))) (should (equal nil (and-let* ((x nil))))) (should (equal 1 (and-let* ((x 1))))) (should-error (and-let* (nil (x 1))) :type 'setting-constant) (should (equal nil (and-let* ((nil) (x 1))))) (should-error (and-let* (2 (x 1))) :type 'wrong-type-argument) (should (equal 1 (and-let* ((2) (x 1))))) (should (equal 2 (and-let* ((x 1) (2))))) (should (equal nil (let ((x nil)) (and-let* (x) x)))) (should (equal "" (let ((x "")) (and-let* (x) x)))) (should (equal "" (let ((x "")) (and-let* (x))))) (should (equal 2 (let ((x 1)) (and-let* (x) (+ x 1))))) (should (equal nil (let ((x nil)) (and-let* (x) (+ x 1))))) (should (equal 2 (let ((x 1)) (and-let* (((> x 0))) (+ x 1))))) (should (equal t (let ((x 1)) (and-let* (((> x 0))))))) (should (equal nil (let ((x 0)) (and-let* (((> x 0))) (+ x 1))))) (should (equal 3 (let ((x 1)) (and-let* (((> x 0)) (x (+ x 1))) (+ x 1)))))) (ert-deftest srfi-2-test-rebind () (should (equal 4 (let ((x 1)) (and-let* (((> x 0)) (x (+ x 1)) (x (+ x 1))) (+ x 1)))))) (ert-deftest srfi-2-test-group-2 () (should (equal 2 (let ((x 1)) (and-let* (x ((> x 0))) (+ x 1))))) (should (equal 2 (let ((x 1)) (and-let* (((progn x)) ((> x 0))) (+ x 1))))) (should (equal nil (let ((x 0)) (and-let* (x ((> x 0))) (+ x 1))))) (should (equal nil (let ((x nil)) (and-let* (x ((> x 0))) (+ x 1))))) (should (equal nil (let ((x nil)) (and-let* (((progn x)) ((> x 0))) (+ x 1)))))) (ert-deftest srfi-2-test-group-3 () (should (equal nil (let ((x 1)) (and-let* (x (y (- x 1)) ((> y 0))) (/ x y))))) (should (equal nil (let ((x 0)) (and-let* (x (y (- x 1)) ((> y 0))) (/ x y))))) (should (equal nil (let ((x nil)) (and-let* (x (y (- x 1)) ((> y 0))) (/ x y))))) (should (equal (/ 3.0 2) (let ((x 3.0)) (and-let* (x (y (- x 1)) ((> y 0))) (/ x y)))))) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-08-27 20:11 bug#28254: 26.0.50; SRFI-2 and-let* Mark Oteiza @ 2017-09-01 2:55 ` npostavs 2017-09-02 2:10 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: npostavs @ 2017-09-01 2:55 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254 Mark Oteiza <mvoteiza@udel.edu> writes: > its behavior overlaps with when-let* but each has things the other > lacks. Can the differences be minimized? I think having a bunch of tiny differences is not good for reader comprehension or code reuse. How much would be lost if we implemented and-let* like this? (defmacro and-let* (varlist &rest body) `(when-let* ,varlist ,@(or body '(t)))) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-01 2:55 ` npostavs @ 2017-09-02 2:10 ` Mark Oteiza 2017-09-02 3:05 ` npostavs 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-02 2:10 UTC (permalink / raw) To: npostavs; +Cc: 28254 On 31/08/17 at 10:55pm, npostavs@users.sourceforge.net wrote: >Mark Oteiza <mvoteiza@udel.edu> writes: > >> its behavior overlaps with when-let* but each has things the other >> lacks. > >Can the differences be minimized? I think having a bunch of tiny >differences is not good for reader comprehension or code reuse. Understood. I was leaning toward putting and-let* in its own file because of the differences instead of having them all in the same place. I'd rather have no and-let* than a gimped and-let* (I shouldn't have made the alias, in retrospect). >How much would be lost if we implemented and-let* like this? > >(defmacro and-let* (varlist &rest body) > `(when-let* ,varlist > ,@(or body '(t)))) The (EXPR) part of it, e.g. (and-let* ((x ...) (y ...) ((x > y))) ...) where the body gets executed if x > y. I didn't try writing it in the style of if-let*--perhaps if-let* could be extended and all three macros would learn (EXPR). if-let* already has the single tuple quirk which differs from most (if not all?) lets in Elisp. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 2:10 ` Mark Oteiza @ 2017-09-02 3:05 ` npostavs 2017-09-02 4:14 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: npostavs @ 2017-09-02 3:05 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254 Mark Oteiza <mvoteiza@udel.edu> writes: > (and-let* ((x ...) > (y ...) > ((x > y))) > ...) I assume you meant (> x y), unless you've managed to somehow squeeze an infix expression parser into that patch above ;) > I didn't try writing it in the style of if-let*--perhaps if-let* could > be extended and all three macros would learn (EXPR). Yes, I think it's best if all foo-let* macros interpret the varlist in the same way. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 3:05 ` npostavs @ 2017-09-02 4:14 ` Mark Oteiza 2017-09-02 5:25 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-02 4:14 UTC (permalink / raw) To: npostavs; +Cc: 28254 On 01/09/17 at 11:05pm, npostavs@users.sourceforge.net wrote: >Mark Oteiza <mvoteiza@udel.edu> writes: > >> (and-let* ((x ...) >> (y ...) >> ((x > y))) >> ...) > >I assume you meant (> x y), unless you've managed to somehow squeeze an >infix expression parser into that patch above ;) Ah yeah, I wish! >> I didn't try writing it in the style of if-let*--perhaps if-let* could >> be extended and all three macros would learn (EXPR). > >Yes, I think it's best if all foo-let* macros interpret the varlist in >the same way. Alright, I'll look at it. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 4:14 ` Mark Oteiza @ 2017-09-02 5:25 ` Michael Heerdegen 2017-09-02 13:36 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-02 5:25 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, npostavs Mark Oteiza <mvoteiza@udel.edu> writes: > >> I didn't try writing it in the style of if-let*--perhaps if-let* could > >> be extended and all three macros would learn (EXPR). > > > >Yes, I think it's best if all foo-let* macros interpret the varlist in > >the same way. > > Alright, I'll look at it. Isn't there a problem with EXPR being a symbol S, which already has a different meaning (bind S to nil)? Though, this seems barely useful to me. Anyway, introducing (EXPR) would thus be backward incompatible. Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 5:25 ` Michael Heerdegen @ 2017-09-02 13:36 ` Mark Oteiza 2017-09-02 18:41 ` Noam Postavsky 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-02 13:36 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, npostavs On 02/09/17 at 07:25am, Michael Heerdegen wrote: >Mark Oteiza <mvoteiza@udel.edu> writes: > >> >> I didn't try writing it in the style of if-let*--perhaps if-let* could >> >> be extended and all three macros would learn (EXPR). >> > >> >Yes, I think it's best if all foo-let* macros interpret the varlist in >> >the same way. >> >> Alright, I'll look at it. > >Isn't there a problem with EXPR being a symbol S, which already has a >different meaning (bind S to nil)? Though, this seems barely useful to >me. Anyway, introducing (EXPR) would thus be backward incompatible. Yeah, that is true. The following patch implements most of the previous, except it doesn't specially handle EXPR being just a symbol. All the incumbent subr-x-tests pass. This single tuple special case is troublesome IMO: (if-let* (x) "dogs" "cats") => "cats" (if-let* (x (y 2)) "dogs" "cats") => (void-function y) (if-let* (x (y 1) (z 2)) "dogs" "cats") => "cats" I'm curious if this was brought up in the old discussion when this was implemented. diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 849ac19d6a..eeacdbcfcd 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -83,10 +83,16 @@ thread-last `(internal--thread-argument nil ,@forms)) (defsubst internal--listify (elt) - "Wrap ELT in a list if it is not one." - (if (not (listp elt)) - (list elt) - elt)) + "Wrap ELT in a list if it is not one. +If ELT is of the form ((EXPR)), listify (EXPR) with a dummy symbol." + (cond + ((nlistp elt) (list elt)) + ((atom (car elt)) elt) + ((and (null (cdr elt)) + (let ((form (car elt))) + (or (listp form) (atom form)))) + (list (cl-gensym) (car elt))) + (t elt))) (defsubst internal--check-binding (binding) "Check BINDING is properly formed." @@ -122,8 +128,11 @@ if-let* Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of THEN is returned, or the last form in ELSE is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST THEN ELSE...)" @@ -134,18 +143,23 @@ if-let* (not (listp (car bindings)))) ;; Adjust the single binding case (setq bindings (list bindings))) - `(let* ,(internal--build-bindings bindings) - (if ,(car (internal--listify (car (last bindings)))) - ,then - ,@else))) + (if bindings + `(let* ,(setq bindings (internal--build-bindings bindings)) + (if ,(caar (last bindings)) + ,then + ,@else)) + `(let* () ,then))) (defmacro when-let* (bindings &rest body) "Bind variables according to VARLIST and conditionally eval BODY. Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of the last form in BODY is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST BODY...)" @@ -154,7 +168,12 @@ when-let* (defalias 'if-let 'if-let*) (defalias 'when-let 'when-let*) -(defalias 'and-let* 'when-let*) + +(defmacro and-let* (varlist &rest body) + "Bind variables according to VARLIST and conditionally eval BODY. +Like `when-let*', except if BODY is empty and all the bindings +are non-nil, then the result is t." + `(when-let* ,varlist ,@(or body '(t)))) (defsubst hash-table-empty-p (hash-table) "Check whether HASH-TABLE is empty (has 0 elements)." ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 13:36 ` Mark Oteiza @ 2017-09-02 18:41 ` Noam Postavsky 2017-09-03 17:48 ` Michael Heerdegen 2017-09-04 1:13 ` Mark Oteiza 0 siblings, 2 replies; 33+ messages in thread From: Noam Postavsky @ 2017-09-02 18:41 UTC (permalink / raw) To: Mark Oteiza; +Cc: Michael Heerdegen, 28254 On Sat, Sep 2, 2017 at 9:36 AM, Mark Oteiza <mvoteiza@udel.edu> wrote: > On 02/09/17 at 07:25am, Michael Heerdegen wrote: >> >> Isn't there a problem with EXPR being a symbol S, which already has a >> different meaning (bind S to nil)? Though, this seems barely useful to >> me. Anyway, introducing (EXPR) would thus be backward incompatible. What would be the point of binding S to nil? In the foo-let macros that would be equivalent to just putting nil (if non-list EXPRs are supported), no? > This single tuple special case is troublesome IMO: > > (if-let* (x) "dogs" "cats") => "cats" > (if-let* (x (y 2)) "dogs" "cats") => (void-function y) > (if-let* (x (y 1) (z 2)) "dogs" "cats") => "cats" > > I'm curious if this was brought up in the old discussion when this was > implemented. I think I'd be okay with dropping support for the S = (S nil) thing in foo-let macros, so that all of the above would give (void-variable x). Although perhaps the incompatibility with plain let would be annoying? To be honest I hardly ever make use of S = (S nil) in plain let either so it wouldn't hit me at all. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 18:41 ` Noam Postavsky @ 2017-09-03 17:48 ` Michael Heerdegen 2017-09-03 22:39 ` Mark Oteiza 2017-09-04 1:13 ` Mark Oteiza 1 sibling, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-03 17:48 UTC (permalink / raw) To: Noam Postavsky; +Cc: Mark Oteiza, 28254 Noam Postavsky <npostavs@users.sourceforge.net> writes: > On Sat, Sep 2, 2017 at 9:36 AM, Mark Oteiza <mvoteiza@udel.edu> wrote: > > On 02/09/17 at 07:25am, Michael Heerdegen wrote: > >> > >> Isn't there a problem with EXPR being a symbol S, which already has a > >> different meaning (bind S to nil)? Though, this seems barely > >> useful to > >> me. Anyway, introducing (EXPR) would thus be backward incompatible. > > What would be the point of binding S to nil? In the foo-let macros > that would be equivalent to just putting nil (if non-list EXPRs are > supported), no? Eh hmm - yes, I think so. It isn't useful. > I think I'd be okay with dropping support for the S = (S nil) thing in > foo-let macros, so that all of the above would give (void-variable x). > Although perhaps the incompatibility with plain let would be annoying? > To be honest I hardly ever make use of S = (S nil) in plain let either > so it wouldn't hit me at all. I think the main use case is to declare a local variable when you don't care about the init value. In the case of if-let, S = (S nil) is not useful, since you can't use that binding neither in the "then" clause (because it won't be executed) nor in the "else" clauses (which ignore all bindings). Even if an `if-let' form is the result of a macro expansion, the S = (S nil) case isn't of any value. So I see no reasons to not drop support for it. Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-03 17:48 ` Michael Heerdegen @ 2017-09-03 22:39 ` Mark Oteiza 2017-09-04 0:48 ` Mark Oteiza ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Mark Oteiza @ 2017-09-03 22:39 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 03/09/17 at 07:48pm, Michael Heerdegen wrote: >Noam Postavsky <npostavs@users.sourceforge.net> writes: > >> I think I'd be okay with dropping support for the S = (S nil) thing in >> foo-let macros, so that all of the above would give (void-variable x). >> Although perhaps the incompatibility with plain let would be annoying? >> To be honest I hardly ever make use of S = (S nil) in plain let either >> so it wouldn't hit me at all. > >I think the main use case is to declare a local variable when you don't >care about the init value. In the case of if-let, S = (S nil) is not >useful, since you can't use that binding neither in the "then" clause >(because it won't be executed) nor in the "else" clauses (which ignore >all bindings). > >Even if an `if-let' form is the result of a macro expansion, the S = (S >nil) case isn't of any value. So I see no reasons to not drop support >for it. If I'm understanding correctly, it is being agreed that (let ((x 1)) (and-let* (x) x)) ;; => 1 because the macro expands to (let* ((x (and t x))) (if x x)) The following patch achieves this, though it breaks some existing subr-x tests which I haven't yet looked at carefully. diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 849ac19d6a..ec1990110a 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -83,10 +83,22 @@ thread-last `(internal--thread-argument nil ,@forms)) (defsubst internal--listify (elt) - "Wrap ELT in a list if it is not one." - (if (not (listp elt)) - (list elt) - elt)) + "Wrap ELT in a list if it is not one. +If ELT is of the form ((EXPR)), listify (EXPR) with a dummy symbol." + (message "%S" elt) + (cond + ;; could this be cleaner? + ((null elt) (list elt)) + ((symbolp elt) (list elt elt)) + ((nlistp elt) (list elt)) + ((and (null (cdr elt)) + (atom (car elt))) + (list (cl-gensym) (car elt))) + ((and (null (cdr elt)) + (let ((form (car elt))) + (or (listp form) (atom form)))) + (list (cl-gensym) (car elt))) + (t elt))) (defsubst internal--check-binding (binding) "Check BINDING is properly formed." @@ -98,7 +110,10 @@ internal--check-binding (defsubst internal--build-binding-value-form (binding prev-var) "Build the conditional value form for BINDING using PREV-VAR." - `(,(car binding) (and ,prev-var ,(cadr binding)))) + (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)))))) (defun internal--build-binding (binding prev-var) "Check and build a single BINDING with PREV-VAR." @@ -122,8 +137,11 @@ if-let* Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of THEN is returned, or the last form in ELSE is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST THEN ELSE...)" @@ -134,27 +152,43 @@ if-let* (not (listp (car bindings)))) ;; Adjust the single binding case (setq bindings (list bindings))) - `(let* ,(internal--build-bindings bindings) - (if ,(car (internal--listify (car (last bindings)))) - ,then - ,@else))) + (if bindings + `(let* ,(setq bindings (internal--build-bindings bindings)) + (if ,(caar (last bindings)) + ,then + ,@else)) + `(let* () ,then))) (defmacro when-let* (bindings &rest body) "Bind variables according to VARLIST and conditionally eval BODY. Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of the last form in BODY is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST BODY...)" - (declare (indent 1) (debug if-let)) + (declare (indent 1) (debug if-let*)) (list 'if-let bindings (macroexp-progn body))) (defalias 'if-let 'if-let*) (defalias 'when-let 'when-let*) -(defalias 'and-let* 'when-let*) + +(defmacro and-let* (varlist &rest body) + "Bind variables according to VARLIST and conditionally eval BODY. +Like `when-let*', except if BODY is empty and all the bindings +are non-nil, then the result is non-nil." + (declare (indent 1) (debug when-let*)) + ;; `(when-let* ,varlist ,@(or body '(t))) + (if varlist + `(let* ,(setq varlist (internal--build-bindings varlist)) + (if ,(caar (last varlist)) + ,@(or body `(,(caar (last varlist)))))) + `(let* () ,@(or body '(t))))) (defsubst hash-table-empty-p (hash-table) "Check whether HASH-TABLE is empty (has 0 elements)." ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-03 22:39 ` Mark Oteiza @ 2017-09-04 0:48 ` Mark Oteiza 2017-09-04 14:12 ` Michael Heerdegen 2017-09-05 3:47 ` Mark Oteiza 2 siblings, 0 replies; 33+ messages in thread From: Mark Oteiza @ 2017-09-04 0:48 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 03/09/17 at 06:39pm, Mark Oteiza wrote: >On 03/09/17 at 07:48pm, Michael Heerdegen wrote: >>Noam Postavsky <npostavs@users.sourceforge.net> writes: >>Even if an `if-let' form is the result of a macro expansion, the S = (S >>nil) case isn't of any value. So I see no reasons to not drop support >>for it. > >If I'm understanding correctly, it is being agreed that > > (let ((x 1)) (and-let* (x) x)) ;; => 1 > >because the macro expands to > > (let* ((x (and t x))) > (if x x)) > >The following patch achieves this, though it breaks some existing subr-x >tests which I haven't yet looked at carefully. The tests fail precisely because of this change--so I'm not bothered aside from having to adjust the test to account for it. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-03 22:39 ` Mark Oteiza 2017-09-04 0:48 ` Mark Oteiza @ 2017-09-04 14:12 ` Michael Heerdegen 2017-09-05 3:47 ` Mark Oteiza 2 siblings, 0 replies; 33+ messages in thread From: Michael Heerdegen @ 2017-09-04 14:12 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254 Mark Oteiza <mvoteiza@udel.edu> writes: > The following patch achieves this, though it breaks some existing subr-x > tests which I haven't yet looked at carefully. I don't seem to be able to apply this patch. I try (shell-command-on-region 1476 5478 patch -d /home/micha/software/emacs/ -p1) (the region is just the patch lines) and get | patching file lisp/emacs-lisp/subr-x.el | patch: **** malformed patch at line 8: (defsubst internal--listify (elt) Any idea why I am seeing this? Thanks, Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-03 22:39 ` Mark Oteiza 2017-09-04 0:48 ` Mark Oteiza 2017-09-04 14:12 ` Michael Heerdegen @ 2017-09-05 3:47 ` Mark Oteiza 2017-09-05 15:04 ` Michael Heerdegen 2017-09-06 12:12 ` Michael Heerdegen 2 siblings, 2 replies; 33+ messages in thread From: Mark Oteiza @ 2017-09-05 3:47 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 03/09/17 at 06:39pm, Mark Oteiza wrote: >On 03/09/17 at 07:48pm, Michael Heerdegen wrote: >>Noam Postavsky <npostavs@users.sourceforge.net> writes: >> >>>I think I'd be okay with dropping support for the S = (S nil) thing in >>>foo-let macros, so that all of the above would give (void-variable x). >>>Although perhaps the incompatibility with plain let would be annoying? >>>To be honest I hardly ever make use of S = (S nil) in plain let either >>>so it wouldn't hit me at all. >> >>I think the main use case is to declare a local variable when you don't >>care about the init value. In the case of if-let, S = (S nil) is not >>useful, since you can't use that binding neither in the "then" clause >>(because it won't be executed) nor in the "else" clauses (which ignore >>all bindings). >> >>Even if an `if-let' form is the result of a macro expansion, the S = (S >>nil) case isn't of any value. So I see no reasons to not drop support >>for it. > >If I'm understanding correctly, it is being agreed that > > (let ((x 1)) (and-let* (x) x)) ;; => 1 > >because the macro expands to > > (let* ((x (and t x))) > (if x x)) > >The following patch achieves this, though it breaks some existing subr-x >tests which I haven't yet looked at carefully. Resending patch, looks like I messed it up: diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 849ac19d6a..ec1990110a 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -83,10 +83,21 @@ thread-last `(internal--thread-argument nil ,@forms)) (defsubst internal--listify (elt) - "Wrap ELT in a list if it is not one." - (if (not (listp elt)) - (list elt) - elt)) + "Wrap ELT in a list if it is not one. +If ELT is of the form ((EXPR)), listify (EXPR) with a dummy symbol." + (message "%S" elt) + (cond + ((null elt) (list elt)) + ((symbolp elt) (list elt elt)) + ((nlistp elt) (list elt)) + ((and (null (cdr elt)) + (atom (car elt))) + (list (cl-gensym) (car elt))) + ((and (null (cdr elt)) + (let ((form (car elt))) + (or (listp form) (atom form)))) + (list (cl-gensym) (car elt))) + (t elt))) (defsubst internal--check-binding (binding) "Check BINDING is properly formed." @@ -98,7 +110,10 @@ internal--check-binding (defsubst internal--build-binding-value-form (binding prev-var) "Build the conditional value form for BINDING using PREV-VAR." - `(,(car binding) (and ,prev-var ,(cadr binding)))) + (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)))))) (defun internal--build-binding (binding prev-var) "Check and build a single BINDING with PREV-VAR." @@ -122,8 +137,11 @@ if-let* Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of THEN is returned, or the last form in ELSE is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST THEN ELSE...)" @@ -134,27 +152,43 @@ if-let* (not (listp (car bindings)))) ;; Adjust the single binding case (setq bindings (list bindings))) - `(let* ,(internal--build-bindings bindings) - (if ,(car (internal--listify (car (last bindings)))) - ,then - ,@else))) + (if bindings + `(let* ,(setq bindings (internal--build-bindings bindings)) + (if ,(caar (last bindings)) + ,then + ,@else)) + `(let* () ,then))) (defmacro when-let* (bindings &rest body) "Bind variables according to VARLIST and conditionally eval BODY. Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of the last form in BODY is returned. + Each element of VARLIST is a symbol (which is bound to nil) or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil. In the special case you only want to bind a single value, VARLIST can just be a plain tuple. \n(fn VARLIST BODY...)" - (declare (indent 1) (debug if-let)) + (declare (indent 1) (debug if-let*)) (list 'if-let bindings (macroexp-progn body))) (defalias 'if-let 'if-let*) (defalias 'when-let 'when-let*) -(defalias 'and-let* 'when-let*) + +(defmacro and-let* (varlist &rest body) + "Bind variables according to VARLIST and conditionally eval BODY. +Like `when-let*', except if BODY is empty and all the bindings +are non-nil, then the result is non-nil." + (declare (indent 1) (debug when-let*)) + ;; `(when-let* ,varlist ,@(or body '(t))) + (if varlist + `(let* ,(setq varlist (internal--build-bindings varlist)) + (if ,(caar (last varlist)) + ,@(or body `(,(caar (last varlist)))))) + `(let* () ,@(or body '(t))))) (defsubst hash-table-empty-p (hash-table) "Check whether HASH-TABLE is empty (has 0 elements)." ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-05 3:47 ` Mark Oteiza @ 2017-09-05 15:04 ` Michael Heerdegen 2017-09-06 12:12 ` Michael Heerdegen 1 sibling, 0 replies; 33+ messages in thread From: Michael Heerdegen @ 2017-09-05 15:04 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254 Mark Oteiza <mvoteiza@udel.edu> writes: > Resending patch, looks like I messed it up: Thanks. Hmm, still getting the same error. Also git apply says "corrupt patch at line 8". Maybe I'm doing something wrong... Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-05 3:47 ` Mark Oteiza 2017-09-05 15:04 ` Michael Heerdegen @ 2017-09-06 12:12 ` Michael Heerdegen 2017-09-06 13:06 ` Mark Oteiza 1 sibling, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-06 12:12 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Hi Mark, I just had a quick look. One detail: > + (cond > + ((null elt) (list elt)) > + ((symbolp elt) (list elt elt)) > + ((nlistp elt) (list elt)) > + ((and (null (cdr elt)) > + (atom (car elt))) > + (list (cl-gensym) (car elt))) > + ((and (null (cdr elt)) > + (let ((form (car elt))) > + (or (listp form) (atom form)))) > + (list (cl-gensym) (car elt))) ^^^^^^^^^ Wouldn't this mean we would have to (require 'cl-lib) (without `eval-when-compile') in subr-x? I'm not sure if that would be acceptable. Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-06 12:12 ` Michael Heerdegen @ 2017-09-06 13:06 ` Mark Oteiza 2017-09-06 19:04 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-06 13:06 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 06/09/17 at 02:12pm, Michael Heerdegen wrote: >Hi Mark, > >I just had a quick look. One detail: > >> + (cond >> + ((null elt) (list elt)) >> + ((symbolp elt) (list elt elt)) >> + ((nlistp elt) (list elt)) >> + ((and (null (cdr elt)) >> + (atom (car elt))) >> + (list (cl-gensym) (car elt))) >> + ((and (null (cdr elt)) >> + (let ((form (car elt))) >> + (or (listp form) (atom form)))) >> + (list (cl-gensym) (car elt))) > ^^^^^^^^^ > >Wouldn't this mean we would have to (require 'cl-lib) (without >`eval-when-compile') in subr-x? I'm not sure if that would be >acceptable. Ah, yes. That will just have to change to (make-symbol "x") or similar. (It made debugging a lot easier) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-06 13:06 ` Mark Oteiza @ 2017-09-06 19:04 ` Michael Heerdegen 0 siblings, 0 replies; 33+ messages in thread From: Michael Heerdegen @ 2017-09-06 19:04 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > (It made debugging a lot easier) Ah, I see - just like the `message' call some lines above that ;-) Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-02 18:41 ` Noam Postavsky 2017-09-03 17:48 ` Michael Heerdegen @ 2017-09-04 1:13 ` Mark Oteiza 2017-09-05 3:55 ` Mark Oteiza 1 sibling, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-04 1:13 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, 28254 On 02/09/17 at 02:41pm, Noam Postavsky wrote: >On Sat, Sep 2, 2017 at 9:36 AM, Mark Oteiza <mvoteiza@udel.edu> wrote: >> This single tuple special case is troublesome IMO: >> >> (if-let* (x) "dogs" "cats") => "cats" >> (if-let* (x (y 2)) "dogs" "cats") => (void-function y) >> (if-let* (x (y 1) (z 2)) "dogs" "cats") => "cats" >> >> I'm curious if this was brought up in the old discussion when this was >> implemented. FWIW, this was brought up in the original thread. https://lists.gnu.org/archive/html/emacs-devel/2014-08/msg00228.html IMO the original suggestion of having if-let and when-let be exclusively single binding, while the starred versions excluding the single binding special case would be more sane. P.S. I just realized I didn't copy the tuple part of if-let* into and-let* in the patch I just sent, and therefore missed the problem this special case causes in tests. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-04 1:13 ` Mark Oteiza @ 2017-09-05 3:55 ` Mark Oteiza 2017-09-09 0:33 ` Mark Oteiza 2017-09-12 12:13 ` Michael Heerdegen 0 siblings, 2 replies; 33+ messages in thread From: Mark Oteiza @ 2017-09-05 3:55 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, 28254 On 03/09/17 at 09:13pm, Mark Oteiza wrote: >On 02/09/17 at 02:41pm, Noam Postavsky wrote: >>On Sat, Sep 2, 2017 at 9:36 AM, Mark Oteiza <mvoteiza@udel.edu> wrote: >>>This single tuple special case is troublesome IMO: >>> >>> (if-let* (x) "dogs" "cats") => "cats" >>> (if-let* (x (y 2)) "dogs" "cats") => (void-function y) >>> (if-let* (x (y 1) (z 2)) "dogs" "cats") => "cats" >>> >>>I'm curious if this was brought up in the old discussion when this was >>>implemented. > >FWIW, this was brought up in the original thread. >https://lists.gnu.org/archive/html/emacs-devel/2014-08/msg00228.html > >IMO the original suggestion of having if-let and when-let be exclusively >single binding, while the starred versions excluding the single binding >special case would be more sane. > >P.S. I just realized I didn't copy the tuple part of if-let* into >and-let* in the patch I just sent, and therefore missed the problem this >special case causes in tests. This is a patch implementing the above: if-let and when-let only take single tuple, while {if,when,and}-let* lose the single tuple special case. diff --git a/etc/NEWS b/etc/NEWS index 2b0c86d7af..24568d637b 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1120,6 +1120,14 @@ be disabled by setting 'byte-compile-cond-use-jump-table' to nil. --- ** The alist 'ucs-names' is now a hash table. +--- +** Semantics of 'if-let', 'when-let', 'if-let*', and 'when-let*' have +changed. 'if-let' and 'when-let' now only accept a single tuple to +bind a single symbol. 'if-let*' and 'when-let*' no longer accept the +single tuple special case. New macro 'and-let*' is an implementation +of the Scheme SRFI-2 syntax of the same name. 'if-let*' and +'when-let*' now accept the same binding syntax as 'and-let*'. + --- ** 'C-up', 'C-down', 'C-left' and 'C-right' are now defined in term mode to send the same escape sequences that xterm does. This makes @@ -1479,10 +1487,6 @@ It avoids unnecessary consing (and garbage collection). +++ ** 'car' and 'cdr' compositions 'cXXXr' and 'cXXXXr' are now part of Elisp. ---- -** 'if-let*', 'when-let*', and 'and-let*' are new in subr-x.el. -The incumbent 'if-let' and 'when-let' are now aliases. - --- ** Low-level list functions like 'length' and 'member' now do a better job of signaling list cycles instead of looping indefinitely. diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 849ac19d6a..e59211a1dc 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -83,10 +83,15 @@ thread-last `(internal--thread-argument nil ,@forms)) (defsubst internal--listify (elt) - "Wrap ELT in a list if it is not one." - (if (not (listp elt)) - (list elt) - elt)) + "Wrap ELT in a list if it is not one. +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)))) + (list (cl-gensym) (car elt))) + (t elt))) (defsubst internal--check-binding (binding) "Check BINDING is properly formed." @@ -98,7 +103,10 @@ internal--check-binding (defsubst internal--build-binding-value-form (binding prev-var) "Build the conditional value form for BINDING using PREV-VAR." - `(,(car binding) (and ,prev-var ,(cadr binding)))) + (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)))))) (defun internal--build-binding (binding prev-var) "Check and build a single BINDING with PREV-VAR." @@ -117,44 +125,62 @@ internal--build-bindings binding)) bindings))) -(defmacro if-let* (bindings then &rest else) +(defmacro if-let* (varlist then &rest else) "Bind variables according to VARLIST and eval THEN or ELSE. Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of THEN is returned, or the last form in ELSE is returned. + Each element of VARLIST is a symbol (which is bound to nil) -or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). -In the special case you only want to bind a single value, -VARLIST can just be a plain tuple. -\n(fn VARLIST THEN ELSE...)" +or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value +of VALUEFORM). +An element can additionally be of the form (EXPR), which is +evaluated and checked for nil." (declare (indent 2) - (debug ([&or (&rest [&or symbolp (symbolp form)]) (symbolp form)] + (debug ((&rest [&or symbolp (symbolp form) (sexp)]) form body))) - (when (and (<= (length bindings) 2) - (not (listp (car bindings)))) - ;; Adjust the single binding case - (setq bindings (list bindings))) - `(let* ,(internal--build-bindings bindings) - (if ,(car (internal--listify (car (last bindings)))) - ,then - ,@else))) + (if varlist + `(let* ,(setq varlist (internal--build-bindings varlist)) + (if ,(caar (last varlist)) + ,then + ,@else)) + `(let* () ,@else))) -(defmacro when-let* (bindings &rest body) +(defmacro when-let* (varlist &rest body) "Bind variables according to VARLIST and conditionally eval BODY. Each binding is evaluated in turn with `let*', and evaluation stops if a binding value is nil. If all are non-nil, the value of the last form in BODY is returned. -Each element of VARLIST is a symbol (which is bound to nil) -or a list (SYMBOL VALUEFORM) (which binds SYMBOL to the value of VALUEFORM). -In the special case you only want to bind a single value, -VARLIST can just be a plain tuple. -\n(fn VARLIST BODY...)" + +VARLIST is the same as in `if-let*'." + (declare (indent 1) (debug if-let*)) + (list 'if-let* varlist (macroexp-progn body))) + +(defmacro and-let* (varlist &rest body) + "Bind variables according to VARLIST and conditionally eval BODY. +Like `when-let*', except if BODY is empty and all the bindings +are non-nil, then the result is non-nil." + (declare (indent 1) (debug when-let*)) + (let (res) + (if varlist + `(let* ,(setq varlist (internal--build-bindings varlist)) + (if ,(setq res (caar (last varlist))) + ,@(or body `(,res)))) + `(let* () ,@(or body '(t)))))) + +(defmacro if-let (spec then &rest else) + "Bind variables according to SPEC and eval THEN or ELSE. +Like `if-let*' except SPEC is of the form (SYMBOL VALUEFORM)" + (declare (indent 2) (debug ((symbolp form) form body))) + (if spec + `(let (,spec) (if ,(car spec) ,then ,@else)) + `(let () ,@else))) + +(defmacro when-let (spec &rest body) + "Bind variables according to SPEC and conditionally eval BODY. +Like `when-let*' except SPEC is of the form (SYMBOL VALUEFORM)" (declare (indent 1) (debug if-let)) - (list 'if-let bindings (macroexp-progn body))) - -(defalias 'if-let 'if-let*) -(defalias 'when-let 'when-let*) -(defalias 'and-let* 'when-let*) + (list 'if-let spec (macroexp-progn body))) (defsubst hash-table-empty-p (hash-table) "Check whether HASH-TABLE is empty (has 0 elements)." ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-05 3:55 ` Mark Oteiza @ 2017-09-09 0:33 ` Mark Oteiza 2017-09-12 12:39 ` Michael Heerdegen 2017-09-12 12:13 ` Michael Heerdegen 1 sibling, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-09 0:33 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, 28254 On 04/09/17 at 11:55pm, Mark Oteiza wrote: >On 03/09/17 at 09:13pm, Mark Oteiza wrote: >>On 02/09/17 at 02:41pm, Noam Postavsky wrote: >>>On Sat, Sep 2, 2017 at 9:36 AM, Mark Oteiza <mvoteiza@udel.edu> wrote: >>>>This single tuple special case is troublesome IMO: >>>> >>>>(if-let* (x) "dogs" "cats") => "cats" >>>>(if-let* (x (y 2)) "dogs" "cats") => (void-function y) >>>>(if-let* (x (y 1) (z 2)) "dogs" "cats") => "cats" >>>> >>>>I'm curious if this was brought up in the old discussion when this was >>>>implemented. >> >>FWIW, this was brought up in the original thread. >>https://lists.gnu.org/archive/html/emacs-devel/2014-08/msg00228.html >> >>IMO the original suggestion of having if-let and when-let be exclusively >>single binding, while the starred versions excluding the single binding >>special case would be more sane. >> >>P.S. I just realized I didn't copy the tuple part of if-let* into >>and-let* in the patch I just sent, and therefore missed the problem this >>special case causes in tests. > >This is a patch implementing the above: if-let and when-let only take >single tuple, while {if,when,and}-let* lose the single tuple special >case. Any comments? ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-09 0:33 ` Mark Oteiza @ 2017-09-12 12:39 ` Michael Heerdegen 2017-09-12 13:09 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-12 12:39 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > Any comments? Some notes on the documentation of `if-let*' after your patch: | "Bind variables according to VARLIST and eval THEN or ELSE. | Each binding is evaluated in turn with `let*', Maybe comparing with `let*' is confusing after the change, because not all bindings must look like (SYMBOL EXPRESSION) any more. I think when we just remove the two words "with `let*'", the documentation is still fine. | and evaluation stops if a binding value is nil. If all are non-nil, | the value of THEN is returned, or the last form in ELSE is returned. Not really related to your change, but: Maybe we should additionally say that THEN can refer to the bindings made in the VARLIST, but ELSE to none, not even to those that resulted in non-nil values before "failing". | Each element of VARLIST is a symbol (which is bound to nil) [...] Did we agree that we drop this useless case? Thanks, Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-12 12:39 ` Michael Heerdegen @ 2017-09-12 13:09 ` Mark Oteiza 2017-09-12 18:44 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-12 13:09 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 12/09/17 at 02:39pm, Michael Heerdegen wrote: > Mark Oteiza <mvoteiza@udel.edu> writes: > > > Any comments? > > Some notes on the documentation of `if-let*' after your patch: > > | "Bind variables according to VARLIST and eval THEN or ELSE. > | Each binding is evaluated in turn with `let*', > > Maybe comparing with `let*' is confusing after the change, because not > all bindings must look like (SYMBOL EXPRESSION) any more. I think when > we just remove the two words "with `let*'", the documentation is still > fine. I agree, thanks. > | and evaluation stops if a binding value is nil. If all are non-nil, > | the value of THEN is returned, or the last form in ELSE is returned. > > Not really related to your change, but: Maybe we should additionally say > that THEN can refer to the bindings made in the VARLIST, but ELSE to > none, not even to those that resulted in non-nil values before > "failing". That's not true though--you can refer to the bindings in either branch: (if-let* ((x 2) (y nil)) x (list x y)) => (let* ((x (and t 2)) (y (and x nil))) (if y x (list x y))) > | Each element of VARLIST is a symbol (which is bound to nil) [...] > > Did we agree that we drop this useless case? Ah yes, this is no longer the case in this patch. Good catch, thank you. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-12 13:09 ` Mark Oteiza @ 2017-09-12 18:44 ` Michael Heerdegen 2017-09-12 20:21 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-12 18:44 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > > Not really related to your change, but: Maybe we should additionally > > say that THEN can refer to the bindings made in the VARLIST, but > > ELSE to none, not even to those that resulted in non-nil values > > before "failing". > > That's not true though--you can refer to the bindings in either branch: Hmm, that feels strange. FWIW, all the scheme implementations I looked at implemented it in a way that binding variables stops after the first nil. OTOH I would expect that all bindings are only available from the THIS branch (this is my personal opinion). In our case, we have something third: always all bindings are visible in the ELSEs, e.g. (let ((z 1)) (if-let* ((nil) (z 100)) (doesnt-matter) z)) ==> nil That doesn't feel right. I have two more questions: In `internal--listify': (defsubst internal--listify (elt) "Wrap ELT in a list if it is not one. 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)))) (list (make-symbol "s") (car elt))) (t elt))) isn't (or (listp form) (atom form)) always true? Secondly, in `internal--build-binding-value-form': (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)))))) 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? Thanks, Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-12 18:44 ` Michael Heerdegen @ 2017-09-12 20:21 ` Mark Oteiza 2017-09-13 10:16 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-12 20:21 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 12/09/17 at 08:44pm, Michael Heerdegen wrote: > Mark Oteiza <mvoteiza@udel.edu> writes: > > > > Not really related to your change, but: Maybe we should additionally > > > say that THEN can refer to the bindings made in the VARLIST, but > > > ELSE to none, not even to those that resulted in non-nil values > > > before "failing". > > > > That's not true though--you can refer to the bindings in either branch: > > Hmm, that feels strange. FWIW, all the scheme implementations I looked > at implemented it in a way that binding variables stops after the first > nil. OTOH I would expect that all bindings are only available from the > THIS branch (this is my personal opinion). In our case, we have > something third: always all bindings are visible in the ELSEs, e.g. > > (let ((z 1)) > (if-let* ((nil) (z 100)) > (doesnt-matter) > z)) > > ==> nil > > That doesn't feel right. Yeah, it's a product of how this is written as building a list of shortcircuiting bindings instead of recursively or otherwise--not sure if there's more to it than that. I can't say I fully understood the Guile implementation when I read it last. Worth thinking about. > 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))))) > 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))))) ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-12 20:21 ` Mark Oteiza @ 2017-09-13 10:16 ` Michael Heerdegen 2017-09-13 11:48 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-13 10:16 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky 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)? > > 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. Thanks, Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 10:16 ` Michael Heerdegen @ 2017-09-13 11:48 ` Mark Oteiza 2017-09-13 16:46 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-13 11:48 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky 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." ^ permalink raw reply related [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 11:48 ` Mark Oteiza @ 2017-09-13 16:46 ` Michael Heerdegen 2017-09-13 16:49 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-13 16:46 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > I guess the following is fine then--thanks for finding these. Yes, that's what I meant. If you are sure it's ok, please install, and thanks. Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 16:46 ` Michael Heerdegen @ 2017-09-13 16:49 ` Mark Oteiza 2017-09-13 17:05 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-13 16:49 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254-done, Noam Postavsky On 13/09/17 at 06:46pm, Michael Heerdegen wrote: > Mark Oteiza <mvoteiza@udel.edu> writes: > > > I guess the following is fine then--thanks for finding these. > > Yes, that's what I meant. > > If you are sure it's ok, please install, and thanks. Ok, great. All tests passed, so I installed it. Closing! ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 16:49 ` Mark Oteiza @ 2017-09-13 17:05 ` Michael Heerdegen 2017-09-13 17:28 ` Mark Oteiza 0 siblings, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-13 17:05 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254-done, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > Ok, great. All tests passed, so I installed it. Closing! Thanks, good work. One more small thing I thought about after Stefan's question: We have this sentence in the doc of `if-let*': | An element can additionally be of the form (VALUEFORM), which is | evaluated and checked for nil. Would it ease understanding if we would add something like "i.e. you can omit the SYMBOL if you are only interested in the test result". Oh, and I find we have some inconsistency: I haven't looked how the behavior was before, but I see that `if-let*' (and thus `when-let*') treats an empty VARLIST as failure (the ELSEs are executed). Contrary to `and-let*', which treats it as success. IMO, `and-let*' does what is expected, and we should change `if-let*' to behave accordingly (and as the documentation suggests). WDYT? Thanks, Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 17:05 ` Michael Heerdegen @ 2017-09-13 17:28 ` Mark Oteiza 2017-09-13 17:49 ` Michael Heerdegen 0 siblings, 1 reply; 33+ messages in thread From: Mark Oteiza @ 2017-09-13 17:28 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254-done, Noam Postavsky On 13/09/17 at 07:05pm, Michael Heerdegen wrote: > Mark Oteiza <mvoteiza@udel.edu> writes: > > > Ok, great. All tests passed, so I installed it. Closing! > > Thanks, good work. > > One more small thing I thought about after Stefan's question: We have > this sentence in the doc of `if-let*': > > | An element can additionally be of the form (VALUEFORM), which is > | evaluated and checked for nil. > > Would it ease understanding if we would add something like "i.e. you can > omit the SYMBOL if you are only interested in the test result". I think so. > Oh, and I find we have some inconsistency: I haven't looked how the > behavior was before, but I see that `if-let*' (and thus `when-let*') > treats an empty VARLIST as failure (the ELSEs are executed). Contrary > to `and-let*', which treats it as success. IMO, `and-let*' does what is > expected, and we should change `if-let*' to behave accordingly (and as > the documentation suggests). > > WDYT? Agreed. Pushed a change for both points. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-13 17:28 ` Mark Oteiza @ 2017-09-13 17:49 ` Michael Heerdegen 0 siblings, 0 replies; 33+ messages in thread From: Michael Heerdegen @ 2017-09-13 17:49 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254-done, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > Agreed. Pushed a change for both points. Thanks! Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-05 3:55 ` Mark Oteiza 2017-09-09 0:33 ` Mark Oteiza @ 2017-09-12 12:13 ` Michael Heerdegen 2017-09-12 14:29 ` Mark Oteiza 1 sibling, 1 reply; 33+ messages in thread From: Michael Heerdegen @ 2017-09-12 12:13 UTC (permalink / raw) To: Mark Oteiza; +Cc: 28254, Noam Postavsky Mark Oteiza <mvoteiza@udel.edu> writes: > This is a patch implementing the above: if-let and when-let only take > single tuple, while {if,when,and}-let* lose the single tuple special > case. I wonder if we should mark if-let and when-let obsolete instead. Because it is only a special case of the if-let* and when-let* forms (with only one binding), so it is absolutely redundant. Also, I find the new syntax breaking with the binding-list syntax of let confusing. Finally, it would ease the transition for programmers: the modified if-let and when-let break existing code, and it's not obviously for programmers what's suddenly wrong. Michael. ^ permalink raw reply [flat|nested] 33+ messages in thread
* bug#28254: 26.0.50; SRFI-2 and-let* 2017-09-12 12:13 ` Michael Heerdegen @ 2017-09-12 14:29 ` Mark Oteiza 0 siblings, 0 replies; 33+ messages in thread From: Mark Oteiza @ 2017-09-12 14:29 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 28254, Noam Postavsky On 12/09/17 at 02:13pm, Michael Heerdegen wrote: > Mark Oteiza <mvoteiza@udel.edu> writes: > > > This is a patch implementing the above: if-let and when-let only take > > single tuple, while {if,when,and}-let* lose the single tuple special > > case. > > I wonder if we should mark if-let and when-let obsolete instead. > Because it is only a special case of the if-let* and when-let* forms > (with only one binding), so it is absolutely redundant. Also, I find > the new syntax breaking with the binding-list syntax of let confusing. > Finally, it would ease the transition for programmers: the modified > if-let and when-let break existing code, and it's not obviously for > programmers what's suddenly wrong. Sounds good to me. ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2017-09-13 17:49 UTC | newest] Thread overview: 33+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-27 20:11 bug#28254: 26.0.50; SRFI-2 and-let* Mark Oteiza 2017-09-01 2:55 ` npostavs 2017-09-02 2:10 ` Mark Oteiza 2017-09-02 3:05 ` npostavs 2017-09-02 4:14 ` Mark Oteiza 2017-09-02 5:25 ` Michael Heerdegen 2017-09-02 13:36 ` Mark Oteiza 2017-09-02 18:41 ` Noam Postavsky 2017-09-03 17:48 ` Michael Heerdegen 2017-09-03 22:39 ` Mark Oteiza 2017-09-04 0:48 ` Mark Oteiza 2017-09-04 14:12 ` Michael Heerdegen 2017-09-05 3:47 ` Mark Oteiza 2017-09-05 15:04 ` Michael Heerdegen 2017-09-06 12:12 ` Michael Heerdegen 2017-09-06 13:06 ` Mark Oteiza 2017-09-06 19:04 ` Michael Heerdegen 2017-09-04 1:13 ` Mark Oteiza 2017-09-05 3:55 ` Mark Oteiza 2017-09-09 0:33 ` Mark Oteiza 2017-09-12 12:39 ` Michael Heerdegen 2017-09-12 13:09 ` Mark Oteiza 2017-09-12 18:44 ` Michael Heerdegen 2017-09-12 20:21 ` Mark Oteiza 2017-09-13 10:16 ` Michael Heerdegen 2017-09-13 11:48 ` Mark Oteiza 2017-09-13 16:46 ` Michael Heerdegen 2017-09-13 16:49 ` Mark Oteiza 2017-09-13 17:05 ` Michael Heerdegen 2017-09-13 17:28 ` Mark Oteiza 2017-09-13 17:49 ` Michael Heerdegen 2017-09-12 12:13 ` Michael Heerdegen 2017-09-12 14:29 ` Mark Oteiza
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).