bug#49809: [PATCH] Add macro 'pcase-setq'

bug#49809: [PATCH] Add macro 'pcase-setq'

[Okam via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hello,

This patch adds a `setq`-like equivalent to `pcase-let`.  This is
convenient when one wants the bindings to exist outside of a `let` form.

This macro expands into multiple `setq` calls that are combined where
possible.


     ;; => (1 2 3 4)
     (let (a b c d)
       (pcase-setq a 1
                   b 2
                   `[,c ,d] [3 4])
       (list a b c d))


Please let me know what should be changed.

Thank you.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-macro-pcase-setq.patch --]
[-- Type: text/x-patch; name=0001-Add-macro-pcase-setq.patch, Size: 3567 bytes --]

From 824d3b56b5f4bd1aac1b933c353715155edc7698 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 1 Aug 2021 12:33:14 -0400
Subject: [PATCH] Add macro 'pcase-setq'

* doc/lispref/control.texi: Document this macro.
* lisp/emacs-lisp/pcase.el: Add this macro.

This macro is the 'setq' equivalent of 'pcase-let'.
---
 doc/lispref/control.texi |  4 ++++
 etc/NEWS                 |  4 ++++
 lisp/emacs-lisp/pcase.el | 41 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 5026d0a4d7..a0540005d2 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -1312,6 +1312,10 @@ Destructuring with pcase Patterns
 up being equivalent to @code{dolist} (@pxref{Iteration}).
 @end defmac
 
+@defmac pcase-setq pattern value@dots{}
+Bind variables to values in a @code{setq} form, destructuring each
+@var{value} according to its respective @var{pattern}.
+@end defmac
 
 @node Iteration
 @section Iteration
diff --git a/etc/NEWS b/etc/NEWS
index 95a2c87d05..0f11caf512 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -553,6 +553,10 @@ The new 'cl-type' pattern compares types using 'cl-typep', which allows
 comparing simple types like '(cl-type integer)', as well as forms like
 '(cl-type (integer 0 10))'.
 
+*** New macro 'pcase-setq'
+This macro is the 'setq' equivalent of 'pcase-let', which allows for
+destructuring patterns in a 'setq' form.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517db75..f9665eba97 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -317,6 +317,47 @@ pcase-dolist
          (pcase-let* ((,(car spec) ,tmpvar))
            ,@body)))))
 
+;;;###autoload
+(defmacro pcase-setq (&rest args)
+  "Bind values by destructuring with `pcase'.
+
+\(fn PATTERN VALUE PATTERN VALUE ...)"
+  (declare (debug (&rest [pcase-PAT form])))
+  (let ((results)
+        (pattern)
+        (value))
+    (while args
+      (setq pattern (pop args)
+            value (pop args))
+      (push (if (pcase--trivial-upat-p pattern)
+                (list 'setq pattern value)
+              (pcase-compile-patterns
+               value
+               (list (cons pattern
+                           (lambda (varvals &rest _)
+                             (cons 'setq
+                                   (mapcan (lambda (varval)
+                                             (let ((var (car varval))
+                                                   (val (cadr varval)))
+                                               (list var val)))
+                                           varvals)))))))
+            results))
+
+    ;; Combine multiple calls to `setq' to a single call where we can.
+    (let ((returned-setqs)
+          (combining))
+      (dolist (i results)
+        (if (eq (car i) 'setq)
+            (push (cdr i) combining)
+          (when combining
+            (push `(setq ,@(apply #'append combining))
+                  returned-setqs)
+            (setq combining nil))
+          (push i returned-setqs)))
+      (when combining
+        (push `(setq ,@(apply #'append combining))
+              returned-setqs))
+      (macroexp-progn returned-setqs))))
 
 (defun pcase--trivial-upat-p (upat)
   (and (symbolp upat) (not (memq upat pcase--dontcare-upats))))
-- 
2.25.1

bug#49809: [PATCH] Add macro 'pcase-setq'

[Lars Ingebrigtsen]

Okam <okamsn@protonmail.com> writes:

> This patch adds a `setq`-like equivalent to `pcase-let`.  This is
> convenient when one wants the bindings to exist outside of a `let` form.
>
> This macro expands into multiple `setq` calls that are combined where
> possible.
>
>      ;; => (1 2 3 4)
>      (let (a b c d)
>        (pcase-setq a 1
>                    b 2
>                    `[,c ,d] [3 4])
>        (list a b c d))

Perhaps Stefan has an opinion here; added to the CCs.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> This patch adds a `setq`-like equivalent to `pcase-let`.
> This is convenient when one wants the bindings to exist outside of
> a `let` form.

Thanks.

> This macro expands into multiple `setq` calls that are combined where
> possible.

I don't think we should try and combine them: it's not worth the
code complexity.  Personally I'd even restrict the calling convention to
(pcase-setq PAT VAL), but if you want to accept the more general case
with multiple PAT+VAL, I'd prefer expanding it to a (progn (pcase-setq
PAT1 VAL1) ...).  I think the resulting code would be simpler/cleaner.

> Please let me know what should be changed.

See a few more comments below.

> Subject: [PATCH] Add macro 'pcase-setq'
>
> * doc/lispref/control.texi: Document this macro.
> * lisp/emacs-lisp/pcase.el: Add this macro.

Please include the "section/function" info,
e.g. I'd write the message as:

    * lisp/emacs-lisp/pcase.el (pcase-setq): New macro.

    This macro is the 'setq' equivalent of 'pcase-let'.

    * doc/lispref/control.texi (Destructuring with pcase Patterns): Document it.

> +@defmac pcase-setq pattern value@dots{}
> +Bind variables to values in a @code{setq} form, destructuring each
> +@var{value} according to its respective @var{pattern}.
> +@end defmac

I prefer keeping "bind" for the case where we create new variables
(i.e. let-bindings) rather than for assignments.

> +;;;###autoload
> +(defmacro pcase-setq (&rest args)
> +  "Bind values by destructuring with `pcase'.

Same here.

> +\(fn PATTERN VALUE PATTERN VALUE ...)"
> +  (declare (debug (&rest [pcase-PAT form])))
> +  (let ((results)
> +        (pattern)
> +        (value))
> +    (while args
> +      (setq pattern (pop args)
> +            value (pop args))
> +      (push (if (pcase--trivial-upat-p pattern)
> +                (list 'setq pattern value)
> +              (pcase-compile-patterns
> +               value
> +               (list (cons pattern
> +                           (lambda (varvals &rest _)
> +                             (cons 'setq
> +                                   (mapcan (lambda (varval)
> +                                             (let ((var (car varval))
> +                                                   (val (cadr varval)))
> +                                               (list var val)))
> +                                           varvals)))))))
> +            results))

Looks good.  But could you add a few corresponding tests to
`test/lisp/emacs-lisp/pcase-tests.el`, including tests for things like

    (pcase-setq `(,a ,b) nil)


-- Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Looks good.

How much trouble would it be to provide a `setf' variant?

A problem could be any syntax that is used for both pcase patterns and
place expressions I guess.

Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Heerdegen [2021-08-05 03:02:03] wrote:
> Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> Looks good.
>
> How much trouble would it be to provide a `setf' variant?
>
> A problem could be any syntax that is used for both pcase patterns and
> place expressions I guess.

Adding support for (setf (pcase PAT) VAL) is very easy to do without any
change to pcase or gv machinery.

Adding support for (setf PAT VAL), is a bit harder (and introduces
a risk because of the potential overlap, tho this risk is small: gv
places are normally defined for "eliminators" whereas pcase patterns are
usually defined for "constructors"), but still fairly easy.

Mixing pcase patterns and gv places as in

    (setf `(,a . ,(aref V N)) EXP)

would be the most flexible option but I haven't thought much about how
hard it would be to implement, and I'm not sure it's worth the trouble.


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Adding support for (setf (pcase PAT) VAL) is very easy to do without any
> change to pcase or gv machinery.

Well, except that it has to use a different name than `pcase` since
`pcase` is already a GV place.


        Stefan "oops"

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Mixing pcase patterns and gv places as in
>
>     (setf `(,a . ,(aref V N)) EXP)
>
> would be the most flexible option

Yes, that's what I had in mind.  Also for plain `pcase' I guess.

Maybe we could use an explicit (gv PLACE) pattern that is like SYMBOL
but compares/binds/sets the PLACE instead of the SYMBOL.


Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Yes, that's what I had in mind.  Also for plain `pcase' I guess.
> Maybe we could use an explicit (gv PLACE) pattern that is like SYMBOL
> but compares/binds/sets the PLACE instead of the SYMBOL.

I don't see how that would work.  `pcase` is designed to test and
extract data.  It then makes that data available by giving it names
(local variables).

The SYMBOL pattern doesn't "set" that variable, it creates a fresh new
one, but that operation doesn't exist for gv places (the only thing we
can do there is get and set).

It would make sense for `pcase-setq`, of course, but for `pcase` I just
don't see how that would work (unless you'd want it to work like
`cl-letf`, but that's like dynamically scoped let so I think you'd
be hard pressed to find enough compelling use cases to justify it).


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> (unless you'd want it to work like
> `cl-letf`, but that's like dynamically scoped let so I think you'd
> be hard pressed to find enough compelling use cases to justify it).

Yes, that's what I had in mind.  I sometimes even missed the ability to
create dynamical symbol bindings using pcase patterns.

And no, I have no clue if it would be a good idea.  If you have a bad
feeling about the idea then you are probably right.

Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Okam via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On 8/4/21 7:06 PM, Stefan Monnier wrote:
> I don't think we should try and combine them: it's not worth the
> code complexity.  Personally I'd even restrict the calling convention to
> (pcase-setq PAT VAL), but if you want to accept the more general case
> with multiple PAT+VAL, I'd prefer expanding it to a (progn (pcase-setq
> PAT1 VAL1) ...).  I think the resulting code would be simpler/cleaner.

Done.

>> +@defmac pcase-setq pattern value@dots{}
>> +Bind variables to values in a @code{setq} form, destructuring each
>> +@var{value} according to its respective @var{pattern}.
>> +@end defmac
>
> I prefer keeping "bind" for the case where we create new variables
> (i.e. let-bindings) rather than for assignments.

This was changed to the phrase "assign values to variables".

>
> Looks good.  But could you add a few corresponding tests to
> `test/lisp/emacs-lisp/pcase-tests.el`, including tests for things like
>
>      (pcase-setq `(,a ,b) nil)

Added with others. Do you think that the added tests are sufficient?

Thank you.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-Add-macro-pcase-setq.patch --]
[-- Type: text/x-patch; name=v2-0001-Add-macro-pcase-setq.patch, Size: 4881 bytes --]

From 391bdb4efc25b1c3c521b27d6203a5de173a2d14 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 1 Aug 2021 12:33:14 -0400
Subject: [PATCH v2] Add macro 'pcase-setq'

* lisp/emacs-lisp/pcase.el (pcase-setq): New macro.

This macro is the 'setq' equivalent of 'pcase-let'.

* doc/lispref/control.texi (Destructuring with pcase Patterns):
Document this macro.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-setq): Test this new macro.
---
 doc/lispref/control.texi            |  5 ++++
 etc/NEWS                            |  4 ++++
 lisp/emacs-lisp/pcase.el            | 31 +++++++++++++++++++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 36 +++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 5026d0a4d7..6e4a5234e2 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -1312,6 +1312,11 @@ Destructuring with pcase Patterns
 up being equivalent to @code{dolist} (@pxref{Iteration}).
 @end defmac
 
+@defmac pcase-setq pattern value@dots{}
+Assign values to variables in a @code{setq} form,
+destructuring each @var{value} according to its respective
+@var{pattern}.
+@end defmac
 
 @node Iteration
 @section Iteration
diff --git a/etc/NEWS b/etc/NEWS
index 95a2c87d05..0f11caf512 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -553,6 +553,10 @@ The new 'cl-type' pattern compares types using 'cl-typep', which allows
 comparing simple types like '(cl-type integer)', as well as forms like
 '(cl-type (integer 0 10))'.
 
+*** New macro 'pcase-setq'
+This macro is the 'setq' equivalent of 'pcase-let', which allows for
+destructuring patterns in a 'setq' form.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517db75..14af70a65b 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -317,6 +317,37 @@ pcase-dolist
          (pcase-let* ((,(car spec) ,tmpvar))
            ,@body)))))
 
+;;;###autoload
+(defmacro pcase-setq (pat val &rest args)
+  "Assign values to variables by destructuring with `pcase'.
+
+\(fn PATTERN VALUE PATTERN VALUE ...)"
+  (declare (debug (&rest [pcase-PAT form])))
+  (cond
+   (args
+    (let ((arg-length (length args)))
+      (unless (= 0 (mod arg-length 2))
+        (signal 'wrong-number-of-arguments
+                (list 'pcase-setq (+ 2 arg-length)))))
+    (let ((result))
+      (while args
+        (push `(pcase-setq ,(pop args) ,(pop args))
+              result))
+      `(progn
+         (pcase-setq ,pat ,val)
+         ,@(nreverse result))))
+   ((pcase--trivial-upat-p pat)
+    `(setq ,pat ,val))
+   (t
+    (pcase-compile-patterns
+     val
+     (list (cons pat
+                 (lambda (varvals &rest _)
+                   `(setq ,@(mapcan (lambda (varval)
+                                      (let ((var (car varval))
+                                            (val (cadr varval)))
+                                        (list var val)))
+                                    varvals)))))))))
 
 (defun pcase--trivial-upat-p (upat)
   (and (symbolp upat) (not (memq upat pcase--dontcare-upats))))
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 02d3878ad0..c53648383a 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -110,4 +110,40 @@ pcase-tests-cl-type
   (should-error (pcase 1
                   ((cl-type notatype) 'integer))))
 
+(ert-deftest pcase-setq ()
+  (should (equal (list nil nil)
+                 (let (a b)
+                   (pcase-setq `(,a ,b) nil)
+                   (list a b))))
+
+  (should (equal '(1 2)
+                 (let (a b)
+                   (pcase-setq `[,a ,b] [1 2])
+                   (list a b))))
+
+  (should (equal '(1 2)
+                 (let (a b)
+                   (pcase-setq a 1 b 2)
+                   (list a b))))
+
+  (should (= 2 (let (a)
+                 (pcase-setq a 1 `(,a) '(2))
+                 a)))
+
+  (should (equal '(nil [1 2 3] 4)
+                 (let (array list-item array-copy)
+                   (pcase-setq (or `(,list-item) array) [1 2 3]
+                               array-copy array
+                               ;; This re-sets `array' to nil.
+                               (or `(,list-item) array) '(4))
+                   (list array array-copy list-item))))
+
+  (let ((a nil))
+    (should-error (pcase-setq a 1 b)
+                  :type '(wrong-number-of-arguments))
+    (should (eq a nil)))
+
+  (should-error (pcase-setq a)
+                :type '(wrong-number-of-arguments)))
+
 ;;; pcase-tests.el ends here.
-- 
2.25.1

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Okam via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs@gnu.org> writes:

> +(defmacro pcase-setq (pat val &rest args)
> +  "Assign values to variables by destructuring with `pcase'.
> +
> +\(fn PATTERN VALUE PATTERN VALUE ...)"

Can we maybe enhance the docstring a bit?  I think we should at least
cover these points:

- The PATTERNs are normal `pcase' patterns, the VALUES are expressions.

- Evaluation happens sequentially as in `setq' (not in parallel)

- When a PATTERN doesn't match it's VALUE, the pair is silently skipped
  (completely, no partial assignments are performed, AFAIU)

Maybe adding a simple example would not be too bad as well?


Thanks,

Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Added with others.  Do you think that the added tests are sufficient?

The new code looks OK to me.  Just one thing, tho:

> +  (should (equal (list nil nil)
> +                 (let (a b)
> +                   (pcase-setq `(,a ,b) nil)
> +                   (list a b))))

The result is the same whether `pcase-setq` assigns nil or doesn't touch
the vars, so this test is not very effective.  I'd rather do:

    (should (equal (list nil nil)
                   (let ((a 'unset)
                         (b 'unset))
                     (pcase-setq `(,a ,b) nil)
                     (list a b))))

But Michael points out that it seems your code won't perform the
assignment if the pattern doesn't match, which I find to be an
odd behavior.

I'd expect a behavior like that of `pcase-let`, instead.


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I'd expect a behavior like that of `pcase-let`, instead.

I don't recall why it was not achievable to make `pcase-let' reliably
error for non-matching patterns.  Would that be possible for
`pcase-setq'?

(What for example would happen if we just wrapped each PAT inside

  (or PAT (guard (error "pcase-setq: PAT doesn't match")))

?)

Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Heerdegen [2021-08-09 02:28:46] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I'd expect a behavior like that of `pcase-let`, instead.
> I don't recall why it was not achievable to make `pcase-let' reliably
> error for non-matching patterns.

The main difficulty is to convince me that it's a good idea.


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> > I don't recall why it was not achievable to make `pcase-let'
> > reliably error for non-matching patterns.
>
> The main difficulty is to convince me that it's a good idea.

Oh - that thing again ;-)

Writing down pcase patterns is a bit more error-prone for quite a few
people than writing other Lisp expressions (agreed?).  If non-matching
patterns don't error and silently just cause undefined behavior, it's
later harder to find out what went wrong and where.  Problems may show
up only much later and then it's harder to locate the cause.

Undefined behavior has no use per se, unless it has a larger impact on
efficiency.

And why again do you think it is not a good idea?


Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> +(defmacro pcase-setq (pat val &rest args)
>> +  "Assign values to variables by destructuring with `pcase'.
>> +
>> +\(fn PATTERN VALUE PATTERN VALUE ...)"

It seems like there was general agreement to adding this, so I've now
applied Earl's patch, with the following addition to the doc string:

> Can we maybe enhance the docstring a bit?  I think we should at least
> cover these points:
>
> - The PATTERNs are normal `pcase' patterns, the VALUES are expressions.
>
> - Evaluation happens sequentially as in `setq' (not in parallel)
>
> - When a PATTERN doesn't match it's VALUE, the pair is silently skipped
>   (completely, no partial assignments are performed, AFAIU)
>
> Maybe adding a simple example would not be too bad as well?

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> The result is the same whether `pcase-setq` assigns nil or doesn't touch
> the vars, so this test is not very effective.  I'd rather do:
>
>     (should (equal (list nil nil)
>                    (let ((a 'unset)
>                          (b 'unset))
>                      (pcase-setq `(,a ,b) nil)
>                      (list a b))))

I've also added this to the tests, but what it actually returns
instead...

> But Michael points out that it seems your code won't perform the
> assignment if the pattern doesn't match, which I find to be an
> odd behavior.
>
> I'd expect a behavior like that of `pcase-let`, instead.

... because I have no opinion here, really -- behaving like `pcase-let'
would be good, but on the other hand, the current behaviour also kinda
sorta makes sense.

So feel free to adjust the behaviour (or not).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > But Michael points out that it seems your code won't perform the
> > assignment if the pattern doesn't match, which I find to be an
> > odd behavior.

I hope that this is even true in all cases.

> > I'd expect a behavior like that of `pcase-let`, instead.
>
> ... because I have no opinion here, really -- behaving like `pcase-let'
> would be good, but on the other hand, the current behaviour also kinda
> sorta makes sense.

Here is something else that is odd:

(let ((a 17)
      (b 17)
      (x 17))
  (pcase-setq (or `((,a) [(,b)])
                  x)
              '((1) [(2)]))
  (list a b x)) ;; ==> (1 2 nil)

The first `or' branch matches, nevertheless is the binding of `x' being
set to (a totally unrelated value) nil which doesn't make much sense.


Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Okam via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On 8/12/21 2:13 AM, Michael Heerdegen wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>>> But Michael points out that it seems your code won't perform the
>>> assignment if the pattern doesn't match, which I find to be an
>>> odd behavior.
>
> I hope that this is even true in all cases.
>
>>> I'd expect a behavior like that of `pcase-let`, instead.
>>
>> ... because I have no opinion here, really -- behaving like `pcase-let'
>> would be good, but on the other hand, the current behaviour also kinda
>> sorta makes sense.
>
> Here is something else that is odd:
>
> (let ((a 17)
>        (b 17)
>        (x 17))
>    (pcase-setq (or `((,a) [(,b)])
>                    x)
>                '((1) [(2)]))
>    (list a b x)) ;; ==> (1 2 nil)
>
> The first `or' branch matches, nevertheless is the binding of `x' being
> set to (a totally unrelated value) nil which doesn't make much sense.
>
>
> Michael.
>


I think that the behavior makes sense for a pattern-matching `let` and
maybe not so much for a destructuring `setq`, which was my intended use
case. I think that it is different because the effects of the
`pcase-setq` form could be less temporary than the effects of the
`pcase-let` form.

If thought of as a destructuring `setq`, then I see the value in
signaling an error when a pattern doesn't match, as you said, but I
guess that would be inconsistent with the value assignments that `or`
generates.

I've attached an example version that would set all of the named
variables to nil before assigning values from the pattern, but I feel
like it is not the best behavior.  Please correct my understanding if
that is not what was meant by "a behavior like that of `pcase-let`".

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pcase-setq-example.el --]
[-- Type: text/x-emacs-lisp; name=pcase-setq-example.el, Size: 1804 bytes --]

(require 'pcase)

(defmacro pcase-setq (pat val &rest args)
  "Assign values to variables by destructuring with `pcase'.

Each PATTERN and VALUE is handled sequentially.  PATTERN is a
normal `pcase' pattern.  VALUE is an expression.

If a PATTERN does not match its VALUE, then the variables in
that PATTERN are set to nil.

\(fn PATTERN VALUE PATTERN VALUE ...)"
  (declare (debug (&rest [pcase-PAT form])))
  (cond
   (args
    (let ((arg-length (length args)))
      (unless (= 0 (mod arg-length 2))
        (signal 'wrong-number-of-arguments
                (list 'pcase-setq (+ 2 arg-length)))))
    (let ((result))
      (while args
        (push `(pcase-setq ,(pop args) ,(pop args))
              result))
      `(progn
         (pcase-setq ,pat ,val)
         ,@(nreverse result))))
   ((pcase--trivial-upat-p pat)
    `(setq ,pat ,val))
   (t
    (let* ((pcase-setq-unique-vars nil)
           (expansion
            (pcase-compile-patterns
             val
             (list (cons pat
                         (lambda (varvals &rest _)
                           (let ((flat-var-val-list))
                             (dolist (varval varvals)
                               (let ((var (car varval))
                                     (val (cadr varval)))
                                 (unless (memq var pcase-setq-unique-vars)
                                   (push var pcase-setq-unique-vars))
                                 (push var flat-var-val-list)
                                 (push val flat-var-val-list)))
                             `(setq ,@(nreverse flat-var-val-list)))))))))
      `(progn
         (setq ,@(mapcan (lambda (x) (list x nil))
                         pcase-setq-unique-vars))
         ,expansion)))))

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>   (pcase-setq (or `((,a) [(,b)])
>                   x)
>               '((1) [(2)]))
>   (list a b x)) ;; ==> (1 2 nil)

If you want to do different things depending on which pattern matches
you should use `pcase` rather than `pcase-setq` (or `pcase-let` for
that matter).


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Undefined behavior has no use per se, unless it has a larger impact on
> efficiency.

There is indeed a significant difference in efficiency.
E.g. the code for (pcase-let ((`(,x ,y) (EXP))) (VAL x y)) looks like:

    byte code:
      args: nil
    0       constant  EXP
    1       call      0
    2       dup       
    3       car-safe  
    4       stack-ref 1
    5       cdr-safe  
    6       dup       
    7       car-safe  
    8       stack-ref 1
    9       cdr-safe  
    10      stack-ref 3
    11      stack-ref 2
    12      constant  VAL
    13      stack-ref 2
    14      stack-ref 2
    15      call      2
    16      return    

Instead of:

    byte code:
      args: nil
    0       constant  EXP
    1       call      0
    2       dup       
    3       consp     
    4       goto-if-nil-else-pop 3
    7       dup       
    8       car-safe  
    9       stack-ref 1
    10      cdr-safe  
    11      dup       
    12      consp     
    13      goto-if-nil-else-pop 2
    16      dup       
    17      car-safe  
    18      stack-ref 1
    19      cdr-safe  
    20      dup       
    21      not       
    22      goto-if-nil-else-pop 1
    25      stack-ref 3
    26      stack-ref 2
    27      constant  VAL
    28      stack-ref 2
    29      stack-ref 2
    30      call      2
    31      discardN-preserve-tos 2
    33:1    discardN-preserve-tos 2
    35:2    discardN-preserve-tos 2
    37:3    return    

For a semantic where the let is skipped when the pattern fails to match
and

    byte code:
      args: nil
    0       constant  EXP
    1       call      0
    2       dup       
    3       consp     
    4       goto-if-nil 3
    7       dup       
    8       car-safe  
    9       stack-ref 1
    10      cdr-safe  
    11      dup       
    12      consp     
    13      goto-if-nil 2
    16      dup       
    17      car-safe  
    18      stack-ref 1
    19      cdr-safe  
    20      dup       
    21      goto-if-nil 1
    24      stack-ref 4
    25      constant  error
    26      constant  "No clause matching `%S'"
    27      stack-ref 2
    28      call      2
    29      return    
    30:1    stack-ref 3
    31      stack-ref 2
    32      constant  VAL
    33      stack-ref 2
    34      stack-ref 2
    35      call      2
    36      return    
    37:2    stack-ref 2
    38      constant  error
    39      constant  "No clause matching `%S'"
    40      stack-ref 2
    41      call      2
    42      return    
    43:3    dup       
    44      constant  error
    45      constant  "No clause matching `%S'"
    46      stack-ref 2
    47      call      2
    48      return    

For the semantics where we signal an error when the pattern fails to match.

> And why again do you think it is not a good idea?

Because if you want different behaviors when the pattern matches and
when it doesn't, you should use `pcase`.


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> >   (pcase-setq (or `((,a) [(,b)])
> >                   x)
> >               '((1) [(2)]))
> >   (list a b x)) ;; ==> (1 2 nil)
>
> If you want to do different things depending on which pattern matches
> you should use `pcase` rather than `pcase-setq` (or `pcase-let` for
> that matter).

Is using a pattern like

   (or `[,a ,b] `[,a ,b ,_])

ok?


Michael.

bug#49809: [PATCH] Add macro 'pcase-setq'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Michael Heerdegen [2021-08-13 04:55:39] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> >   (pcase-setq (or `((,a) [(,b)])
>> >                   x)
>> >               '((1) [(2)]))
>> >   (list a b x)) ;; ==> (1 2 nil)
>>
>> If you want to do different things depending on which pattern matches
>> you should use `pcase` rather than `pcase-setq` (or `pcase-let` for
>> that matter).
>
> Is using a pattern like
>
>    (or `[,a ,b] `[,a ,b ,_])
>
> ok?

Of course, tho it does the same thing as just `[,a ,b], only
less efficiently.


        Stefan

bug#49809: [PATCH] Add macro 'pcase-setq'

[Michael Heerdegen]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> > Is using a pattern like
> >
> >    (or `[,a ,b] `[,a ,b ,_])
> >
> > ok?
>
> Of course, tho it does the same thing as just `[,a ,b], only
> less efficiently.

I see.


Michael.