GNU bug report logs - #49809
[PATCH] Add macro 'pcase-setq'

Previous Next

Package: emacs;

Reported by: Okam <okamsn <at> protonmail.com>

Date: Sun, 1 Aug 2021 17:21:01 UTC

Severity: normal

Tags: patch

Fixed in version 28.1

Done: Lars Ingebrigtsen <larsi <at> gnus.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49809 in the body.
You can then email your comments to 49809 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Sun, 01 Aug 2021 17:21:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Okam <okamsn <at> protonmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 01 Aug 2021 17:21:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Okam <okamsn <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add macro 'pcase-setq'
Date: Sun, 01 Aug 2021 17:20:41 +0000
[Message part 1 (text/plain, inline)]
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.
[0001-Add-macro-pcase-setq.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Wed, 04 Aug 2021 07:15:02 GMT) Full text and rfc822 format available.

Message #8 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Okam <okamsn <at> protonmail.com>
Cc: 49809 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Wed, 04 Aug 2021 09:14:16 +0200
Okam <okamsn <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Wed, 04 Aug 2021 23:07:01 GMT) Full text and rfc822 format available.

Message #11 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Okam <okamsn <at> protonmail.com>
Cc: 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Wed, 04 Aug 2021 19:06:11 -0400
> 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 <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 05 Aug 2021 01:03:01 GMT) Full text and rfc822 format available.

Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 05 Aug 2021 03:02:03 +0200
Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 05 Aug 2021 01:03:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 05 Aug 2021 13:36:02 GMT) Full text and rfc822 format available.

Message #20 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 05 Aug 2021 09:34:55 -0400
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 <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 05 Aug 2021 15:01:02 GMT) Full text and rfc822 format available.

Message #23 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 05 Aug 2021 11:00:08 -0400
> 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"





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 01:43:02 GMT) Full text and rfc822 format available.

Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 06 Aug 2021 03:42:16 +0200
Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 01:43:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 04:08:01 GMT) Full text and rfc822 format available.

Message #32 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 06 Aug 2021 00:07:07 -0400
> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 04:29:02 GMT) Full text and rfc822 format available.

Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 06 Aug 2021 06:28:06 +0200
Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 04:29:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 06 Aug 2021 22:34:02 GMT) Full text and rfc822 format available.

Message #41 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Okam <okamsn <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 06 Aug 2021 22:33:36 +0000
[Message part 1 (text/plain, inline)]
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 <at> 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.


[v2-0001-Add-macro-pcase-setq.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Sat, 07 Aug 2021 02:13:01 GMT) Full text and rfc822 format available.

Message #44 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Okam via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Sat, 07 Aug 2021 04:11:55 +0200
Okam via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> 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.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Sat, 07 Aug 2021 02:13:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Sat, 07 Aug 2021 15:43:02 GMT) Full text and rfc822 format available.

Message #50 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Okam <okamsn <at> protonmail.com>
Cc: 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Sat, 07 Aug 2021 11:42:09 -0400
> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Mon, 09 Aug 2021 00:30:01 GMT) Full text and rfc822 format available.

Message #53 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Mon, 09 Aug 2021 02:28:46 +0200
Stefan Monnier <monnier <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Mon, 09 Aug 2021 12:52:02 GMT) Full text and rfc822 format available.

Message #56 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Mon, 09 Aug 2021 08:51:21 -0400
Michael Heerdegen [2021-08-09 02:28:46] wrote:
> Stefan Monnier <monnier <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Tue, 10 Aug 2021 03:14:01 GMT) Full text and rfc822 format available.

Message #59 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Tue, 10 Aug 2021 05:13:11 +0200
Stefan Monnier <monnier <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Wed, 11 Aug 2021 21:58:01 GMT) Full text and rfc822 format available.

Message #62 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Wed, 11 Aug 2021 23:57:29 +0200
Michael Heerdegen <michael_heerdegen <at> 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 <at> 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 marked as fixed in version 28.1, send any further explanations to 49809 <at> debbugs.gnu.org and Okam <okamsn <at> protonmail.com> Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Wed, 11 Aug 2021 21:58:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 12 Aug 2021 06:14:02 GMT) Full text and rfc822 format available.

Message #67 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 12 Aug 2021 08:13:36 +0200
Lars Ingebrigtsen <larsi <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 12 Aug 2021 12:12:02 GMT) Full text and rfc822 format available.

Message #70 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Okam <okamsn <at> protonmail.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 49809 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 12 Aug 2021 12:11:22 +0000
[Message part 1 (text/plain, inline)]
On 8/12/21 2:13 AM, Michael Heerdegen wrote:
> Lars Ingebrigtsen <larsi <at> 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`".
[pcase-setq-example.el (text/x-emacs-lisp, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 12 Aug 2021 15:07:02 GMT) Full text and rfc822 format available.

Message #73 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 12 Aug 2021 11:06:36 -0400
>   (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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Thu, 12 Aug 2021 16:15:02 GMT) Full text and rfc822 format available.

Message #76 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: Okam <okamsn <at> protonmail.com>, 49809 <at> debbugs.gnu.org
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Thu, 12 Aug 2021 12:13:58 -0400
> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 13 Aug 2021 02:56:01 GMT) Full text and rfc822 format available.

Message #79 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 13 Aug 2021 04:55:39 +0200
Stefan Monnier <monnier <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 13 Aug 2021 05:18:01 GMT) Full text and rfc822 format available.

Message #82 received at 49809 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org,
 Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 13 Aug 2021 01:17:23 -0400
Michael Heerdegen [2021-08-13 04:55:39] wrote:
> Stefan Monnier <monnier <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 13 Aug 2021 05:27:01 GMT) Full text and rfc822 format available.

Message #85 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: okamsn <at> protonmail.com, 49809 <at> debbugs.gnu.org,
 Lars Ingebrigtsen <larsi <at> gnus.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#49809: [PATCH] Add macro 'pcase-setq'
Date: Fri, 13 Aug 2021 07:26:03 +0200
Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#49809; Package emacs. (Fri, 13 Aug 2021 05:27:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 10 Sep 2021 11:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 201 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.