unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] pcase.el: Add type pattern
@ 2020-03-09 18:19 Adam Porter
  2020-03-09 18:38 ` Eli Zaretskii
                   ` (3 more replies)
  0 siblings, 4 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-09 18:19 UTC (permalink / raw)
  To: emacs-devel

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

Hi,

It seemed like an obvious and useful improvement to add a "type" pattern
to pcase, so I prepared this patch.  I wondered if checking for
predicates like this was a good way to do so, and I saw that the
cl-typep inliner does basically this, so it seems reasonable.

If this seems like an acceptable idea, please let me know if any further
changes are required.

Thanks,
Adam

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern --]
[-- Type: text/x-diff, Size: 2144 bytes --]

From 15248b62ada8e7d5c5484a0dd4749e057b0b8061 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.
---
 etc/NEWS                            | 7 +++++++
 lisp/emacs-lisp/pcase.el            | 8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 7 +++++++
 3 files changed, 22 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..9190bf1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+A pattern like '(type integer)' is equivalent to one like
+'(pred integerp)'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 36b93fa..4a2903b 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -977,5 +977,13 @@ \`
    ;; compounded values that are not `consp'
    (t (error "Unknown QPAT: %S" qpat))))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE."
+  (let* ((type (symbol-name type))
+	 (pred (or (intern-soft (concat type "p"))
+		   (intern-soft (concat type "-p"))
+		   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..5db8605 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,13 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:19 [PATCH] pcase.el: Add type pattern Adam Porter
@ 2020-03-09 18:38 ` Eli Zaretskii
  2020-03-09 19:20   ` Adam Porter
  2020-03-09 18:38 ` Adam Porter
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 36+ messages in thread
From: Eli Zaretskii @ 2020-03-09 18:38 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> From: Adam Porter <adam@alphapapa.net>
> Date: Mon, 09 Mar 2020 13:19:48 -0500
> 
> It seemed like an obvious and useful improvement to add a "type" pattern
> to pcase, so I prepared this patch.  I wondered if checking for
> predicates like this was a good way to do so, and I saw that the
> cl-typep inliner does basically this, so it seems reasonable.
> 
> If this seems like an acceptable idea, please let me know if any further
> changes are required.

Thanks for working on this.  I'll let others comment on whether this
should be added, but if it is, it should be documented in the ELisp
manual, like all the other patterns supported by pcase.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:19 [PATCH] pcase.el: Add type pattern Adam Porter
  2020-03-09 18:38 ` Eli Zaretskii
@ 2020-03-09 18:38 ` Adam Porter
  2020-03-09 18:45 ` Eric Abrahamsen
  2020-03-09 18:53 ` Stefan Monnier
  3 siblings, 0 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-09 18:38 UTC (permalink / raw)
  To: emacs-devel

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

Please forgive the tab-indentation in that patch.  I prepared it with a
clean Emacs config, which I expected would use spaces, and the
indentation appeared correct in the buffer.  I used whitespace-cleanup
to replace the tabs with spaces and updated the patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern --]
[-- Type: text/x-diff, Size: 2179 bytes --]

From 2aa7c4a6f651b482c916f7654935231425c3ce2a Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.
---
 etc/NEWS                            | 7 +++++++
 lisp/emacs-lisp/pcase.el            | 8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 7 +++++++
 3 files changed, 22 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..9190bf1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+A pattern like '(type integer)' is equivalent to one like
+'(pred integerp)'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 36b93fa..334eb33 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -977,5 +977,13 @@ \`
    ;; compounded values that are not `consp'
    (t (error "Unknown QPAT: %S" qpat))))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..5db8605 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,13 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:19 [PATCH] pcase.el: Add type pattern Adam Porter
  2020-03-09 18:38 ` Eli Zaretskii
  2020-03-09 18:38 ` Adam Porter
@ 2020-03-09 18:45 ` Eric Abrahamsen
  2020-03-10 15:48   ` Adam Porter
  2020-03-09 18:53 ` Stefan Monnier
  3 siblings, 1 reply; 36+ messages in thread
From: Eric Abrahamsen @ 2020-03-09 18:45 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> Hi,
>
> It seemed like an obvious and useful improvement to add a "type" pattern
> to pcase, so I prepared this patch.  I wondered if checking for
> predicates like this was a good way to do so, and I saw that the
> cl-typep inliner does basically this, so it seems reasonable.

One thing that's nice about cl-typep is that it will correctly identify
instances of subclasses, which can be nice. Dunno how difficult it might
be to work that in there, but it's something to consider. Either way
this is a nice addition.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:19 [PATCH] pcase.el: Add type pattern Adam Porter
                   ` (2 preceding siblings ...)
  2020-03-09 18:45 ` Eric Abrahamsen
@ 2020-03-09 18:53 ` Stefan Monnier
  2020-03-09 19:31   ` Adam Porter
  3 siblings, 1 reply; 36+ messages in thread
From: Stefan Monnier @ 2020-03-09 18:53 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> It seemed like an obvious and useful improvement to add a "type" pattern
> to pcase, so I prepared this patch.

A `type` pattern seems fine, yes.

> I wondered if checking for predicates like this was a good way to do so,

`cl-typep` does it but I think it's a mistake.
We should not rely on such heuristics when we can "do it right",
e.g. with a property along the lines of `cl-deftype-satisfies`.

> +	 (pred (or (intern-soft (concat type "p"))
> +		   (intern-soft (concat type "-p"))

This fails for those cl-defstructs where the `:predicate` was given another
name (or no name at all).

We could circumvent the problem by expanding the (type T) check to
to a call to `cl-typep`.


        Stefan


PS: Arguably, every type symbol should have a corresponding *class*
(currently stored in the `cl--class` symbol for cl-defstruct and eieio
objects), so we should then add a bunch of classes for non-structs
non-eieio types (such as integer, ...) and we could then define
a generic function which takes such a class and returns the predicate to
use as a test for this type.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:38 ` Eli Zaretskii
@ 2020-03-09 19:20   ` Adam Porter
  2020-03-09 19:35     ` Adam Porter
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2020-03-09 19:20 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks for working on this.  I'll let others comment on whether this
> should be added, but if it is, it should be documented in the ELisp
> manual, like all the other patterns supported by pcase.

Thanks, I'll prepare another patch that updates the manual and pcase's
docstring.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:53 ` Stefan Monnier
@ 2020-03-09 19:31   ` Adam Porter
  2020-03-09 21:00     ` Stefan Monnier
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2020-03-09 19:31 UTC (permalink / raw)
  To: emacs-devel

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

>> I wondered if checking for predicates like this was a good way to do
>> so,
>
> `cl-typep` does it but I think it's a mistake.
> We should not rely on such heuristics when we can "do it right",
> e.g. with a property along the lines of `cl-deftype-satisfies`.

Understood, but I'm not sure exactly what you mean, as far as what I
should do in that regard.  I'm not very familiar with cl-lib's
internals.  :)

>> +	 (pred (or (intern-soft (concat type "p"))
>> +		   (intern-soft (concat type "-p"))
>
> This fails for those cl-defstructs where the `:predicate` was given
> another name (or no name at all).

Good catch, thanks.

> We could circumvent the problem by expanding the (type T) check to
> to a call to `cl-typep`.

That would be fine with me, of course, but I was hesitant to use any
`cl-' functions in pcase.el since it doesn't use any and doesn't already
require `cl-lib'.  Let me know if you want me to do that.

> PS: Arguably, every type symbol should have a corresponding *class*
> (currently stored in the `cl--class` symbol for cl-defstruct and eieio
> objects), so we should then add a bunch of classes for non-structs
> non-eieio types (such as integer, ...) and we could then define a
> generic function which takes such a class and returns the predicate to
> use as a test for this type.

Seems like a good idea to me, although the scope of those changes seem
much larger than this patch, and I'm not sure I'm the right person for
that job.

Thanks for your feedback.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 19:20   ` Adam Porter
@ 2020-03-09 19:35     ` Adam Porter
  0 siblings, 0 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-09 19:35 UTC (permalink / raw)
  To: emacs-devel

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

Adam Porter <adam@alphapapa.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Thanks for working on this.  I'll let others comment on whether this
>> should be added, but if it is, it should be documented in the ELisp
>> manual, like all the other patterns supported by pcase.
>
> Thanks, I'll prepare another patch that updates the manual and pcase's
> docstring.

This patch also updates the Elisp manual and the pcase docstring
accordingly.  It does not yet take into account Stefan's feedback, as
I'm awaiting further instructions.

Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern (also updates docs) --]
[-- Type: text/x-diff, Size: 3377 bytes --]

From 51a96e7f53c92d1d9d893054b73b0a0c2577dfbf Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el:
((pcase-defmacro type)): Add 'type' pattern.
(pcase): Update docstring.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.
---
 doc/lispref/control.texi            | 7 +++++++
 etc/NEWS                            | 7 +++++++
 lisp/emacs-lisp/pcase.el            | 9 +++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 7 +++++++
 4 files changed, 30 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index c601e3a..848d6a3 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,13 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}.  The comparison is done
+with a function named @var{typep} or @var{type-p}, whichever is
+defined.
+
+Example: @code{(type integer)}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.
diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..9190bf1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+A pattern like '(type integer)' is equivalent to one like
+'(pred integerp)'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 36b93fa..628e961 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -128,6 +128,7 @@ pcase
                    If a SYMBOL is used twice in the same pattern
                    the second occurrence becomes an `eq'uality test.
   (pred FUN)       matches if FUN called on EXPVAL returns non-nil.
+  (type TYPE)      matches if EXPVAL is of TYPE.
   (app FUN PAT)    matches if FUN called on EXPVAL matches PAT.
   (guard BOOLEXP)  matches if BOOLEXP evaluates to non-nil.
   (let PAT EXPR)   matches if EXPR matches PAT.
@@ -977,5 +978,13 @@ \`
    ;; compounded values that are not `consp'
    (t (error "Unknown QPAT: %S" qpat))))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..5db8605 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,13 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 19:31   ` Adam Porter
@ 2020-03-09 21:00     ` Stefan Monnier
  2020-03-09 21:54       ` Adam Porter
  0 siblings, 1 reply; 36+ messages in thread
From: Stefan Monnier @ 2020-03-09 21:00 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

>> `cl-typep` does it but I think it's a mistake.
>> We should not rely on such heuristics when we can "do it right",
>> e.g. with a property along the lines of `cl-deftype-satisfies`.
> Understood, but I'm not sure exactly what you mean, as far as what I
> should do in that regard.  I'm not very familiar with cl-lib's
> internals.  :)

The property `cl-deftype-satisfies` is very simple:

   (get <TYPE> 'cl-deftype-satisfies)

gives you the predicate you need to call.

So we could use this property or choose a new property name
(without the `cl-` prefix) which works the same way.  Then we need to
add the corresponding list of

    (put '<TYPE> '<PROP> '<PRED>)

somewhere (probably in `subr.el`, tho if we keep the
`cl-deftype-satisfies` name, then it would also fit in
`cl-preload.el`).

>> We could circumvent the problem by expanding the (type T) check to
>> to a call to `cl-typep`.
> That would be fine with me, of course, but I was hesitant to use any
> `cl-' functions in pcase.el since it doesn't use any and doesn't already
> require `cl-lib'.  Let me know if you want me to do that.

Agreed, especially since cl*.el do use `pcase` nowadays, so making
`pcase` use `cl-lib` can introduce some nasty bootstrapping problems.

Another problem of using `cl-typep` as-is is the question whether we
want to include the CL types such as `nil` (the empty type,
traditionally called ⊥ in type theory), `t` (the type of all objects,
traditionally called ⊤ in type theory), `(integer MIN MAX)`, ...

But at the same time, I can't think of a good reason why we should use
a different notion of type than CL's, so I guess it does make sense to
use `cl-typep` (makes the overall system simpler).  To avoid the
circularity, we can simply move the (pcase-defmacro type ...) outside of
`pcase.el` so it can easily depend on both `pcase` and `cl-lib`.
Another option is to do something like

    (pcase-defmacro type (ty)
      (require 'cl-lib)
      `(pred (pcase--flip cl-typep ',ty)))

so `cl-lib` is only required lazily (when we actually expand a `(type
TYPE)` pattern), so the bootstrapping problem shouldn't bite us as long
as neither pcase.el nor cl*.el themselves use such a pattern.

> Seems like a good idea to me, although the scope of those changes seem
> much larger than this patch,

Indeed.  But it's not enormous either (it's only an addition to what we
have, so there's no tricky business with backward compatibility and stuff).

> and I'm not sure I'm the right person for that job.

Based on the fact that it hasn't been done yet, I think we can assume
that noone is "the right person for that job", but you might still be
the closest there is.  I'd be happy to help.


        Stefan




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 21:00     ` Stefan Monnier
@ 2020-03-09 21:54       ` Adam Porter
  2020-03-09 22:22         ` Michael Heerdegen
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2020-03-09 21:54 UTC (permalink / raw)
  To: emacs-devel

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

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

>>> `cl-typep` does it but I think it's a mistake.
>>> We should not rely on such heuristics when we can "do it right",
>>> e.g. with a property along the lines of `cl-deftype-satisfies`.
>> Understood, but I'm not sure exactly what you mean, as far as what I
>> should do in that regard.  I'm not very familiar with cl-lib's
>> internals.  :)
>
> The property `cl-deftype-satisfies` is very simple:
>
>    (get <TYPE> 'cl-deftype-satisfies)
>
> gives you the predicate you need to call.

Thanks, I didn't know about that property.

> So we could use this property or choose a new property name
> (without the `cl-` prefix) which works the same way.  Then we need to
> add the corresponding list of
>
>     (put '<TYPE> '<PROP> '<PRED>)
>
> somewhere (probably in `subr.el`, tho if we keep the
> `cl-deftype-satisfies` name, then it would also fit in
> `cl-preload.el`).

Thanks, now I understand.

> But at the same time, I can't think of a good reason why we should use
> a different notion of type than CL's, so I guess it does make sense to
> use `cl-typep` (makes the overall system simpler).  To avoid the
> circularity, we can simply move the (pcase-defmacro type ...) outside of
> `pcase.el` so it can easily depend on both `pcase` and `cl-lib`.
> Another option is to do something like
>
>     (pcase-defmacro type (ty)
>       (require 'cl-lib)
>       `(pred (pcase--flip cl-typep ',ty)))
>
> so `cl-lib` is only required lazily (when we actually expand a `(type
> TYPE)` pattern), so the bootstrapping problem shouldn't bite us as long
> as neither pcase.el nor cl*.el themselves use such a pattern.

Sounds good to me.  I found that cl-macs.el defines the pcase pattern
for cl-struct, so it seemed like a natural place to put a pattern that
uses cl-typep.  Please see the new patch attached.  I added a
co-authored-by pseudo-header; I hope that's appropriate.

>> Seems like a good idea to me, although the scope of those changes seem
>> much larger than this patch,
>
> Indeed.  But it's not enormous either (it's only an addition to what we
> have, so there's no tricky business with backward compatibility and stuff).
>
>> and I'm not sure I'm the right person for that job.
>
> Based on the fact that it hasn't been done yet, I think we can assume
> that noone is "the right person for that job", but you might still be
> the closest there is.  I'd be happy to help.

If you'd still like me to work on that or the other ideas, please let me
know, and I might be able to find time for that.

Thanks.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern to cl-macs.el, etc. --]
[-- Type: text/x-diff, Size: 3482 bytes --]

From 38598a6b0e940e44d9c5cee84d9a87184c98d051 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/cl-macs.el: Add type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* lisp/emacs-lisp/pcase.el: (pcase): Update docstring.

* doc/lispref/control.texi (pcase Macro): Update manual.

Co-authored-by: Stefan Monnier <monnier@iro.umontreal.ca>
---
 doc/lispref/control.texi            | 6 ++++++
 etc/NEWS                            | 7 +++++++
 lisp/emacs-lisp/cl-macs.el          | 6 ++++++
 lisp/emacs-lisp/pcase.el            | 1 +
 test/lisp/emacs-lisp/pcase-tests.el | 7 +++++++
 5 files changed, 27 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index c601e3a..72bfd2e 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,12 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}.  The comparison is done
+with function @code{cl-typep}.
+
+Example: @code{(type integer)}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.
diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..9190bf1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+A pattern like '(type integer)' is equivalent to one like
+'(pred integerp)'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 4c2f589..d2ac485 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3403,6 +3403,12 @@ cl-struct-slot-value
 
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 36b93fa..a9ef779 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -128,6 +128,7 @@ pcase
                    If a SYMBOL is used twice in the same pattern
                    the second occurrence becomes an `eq'uality test.
   (pred FUN)       matches if FUN called on EXPVAL returns non-nil.
+  (type TYPE)      matches if EXPVAL is of TYPE.
   (app FUN PAT)    matches if FUN called on EXPVAL matches PAT.
   (guard BOOLEXP)  matches if BOOLEXP evaluates to non-nil.
   (let PAT EXPR)   matches if EXPR matches PAT.
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..5db8605 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,13 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 21:54       ` Adam Porter
@ 2020-03-09 22:22         ` Michael Heerdegen
  2020-03-09 22:46           ` Adam Porter
  0 siblings, 1 reply; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-09 22:22 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

>    (pred FUN)       matches if FUN called on EXPVAL returns non-nil.
> +  (type TYPE)      matches if EXPVAL is of TYPE.
>    (app FUN PAT)    matches if FUN called on EXPVAL matches PAT.

I like your idea, but I would not document the new pattern at this
prominent position.  Evaluating the pattern definition will add an entry
in the dynamically created docstring anyway.  I think this is good
enough.

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 22:22         ` Michael Heerdegen
@ 2020-03-09 22:46           ` Adam Porter
  2020-03-10 14:54             ` Michael Heerdegen
  2020-03-10 15:01             ` Michael Heerdegen
  0 siblings, 2 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-09 22:46 UTC (permalink / raw)
  To: emacs-devel

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

>>    (pred FUN)       matches if FUN called on EXPVAL returns non-nil.
>> +  (type TYPE)      matches if EXPVAL is of TYPE.
>>    (app FUN PAT)    matches if FUN called on EXPVAL matches PAT.
>
> I like your idea, but I would not document the new pattern at this
> prominent position.  Evaluating the pattern definition will add an entry
> in the dynamically created docstring anyway.  I think this is good
> enough.

Thanks, I forgot about that part of the docstring.  I removed that line
as you suggested in this updated patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern to cl-macs.el, etc. --]
[-- Type: text/x-diff, Size: 2770 bytes --]

From 628b25d2766884262f7da8ed158d3896f8944913 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/cl-macs.el: Add type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

Co-authored-by: Stefan Monnier <monnier@iro.umontreal.ca>
---
 doc/lispref/control.texi            | 6 ++++++
 etc/NEWS                            | 7 +++++++
 lisp/emacs-lisp/cl-macs.el          | 6 ++++++
 test/lisp/emacs-lisp/pcase-tests.el | 7 +++++++
 4 files changed, 26 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index c601e3a..72bfd2e 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,12 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}.  The comparison is done
+with function @code{cl-typep}.
+
+Example: @code{(type integer)}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.
diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..9190bf1 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,13 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+A pattern like '(type integer)' is equivalent to one like
+'(pred integerp)'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 4c2f589..d2ac485 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3403,6 +3403,12 @@ cl-struct-slot-value
 
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..5db8605 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,13 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 22:46           ` Adam Porter
@ 2020-03-10 14:54             ` Michael Heerdegen
  2020-03-10 15:01             ` Michael Heerdegen
  1 sibling, 0 replies; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-10 14:54 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> Thanks, I forgot about that part of the docstring.  I removed that line
> as you suggested in this updated patch.

Thanks.

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 22:46           ` Adam Porter
  2020-03-10 14:54             ` Michael Heerdegen
@ 2020-03-10 15:01             ` Michael Heerdegen
  2020-03-10 15:46               ` Adam Porter
  1 sibling, 1 reply; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-10 15:01 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> +;;; Pcase type pattern.
> +
> +(pcase-defmacro type (type)
> +  "Pcase pattern that matches objects of TYPE."
> +  `(pred (pcase--flip cl-typep ',type)))

If we keep these semantics and the patch is accepted, you should
document here what TYPE means.  IIUC any type expression allowed by
`cl-typep' will be possible, right?  Then just say something like "TYPE
is a type name like in `cl-typep'" or so.

Thanks,

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 15:01             ` Michael Heerdegen
@ 2020-03-10 15:46               ` Adam Porter
  2020-03-10 16:24                 ` Michael Heerdegen
  2020-03-16 12:59                 ` Stefan Monnier
  0 siblings, 2 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-10 15:46 UTC (permalink / raw)
  To: emacs-devel

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> +(pcase-defmacro type (type)
>> +  "Pcase pattern that matches objects of TYPE."
>> +  `(pred (pcase--flip cl-typep ',type)))
>
> If we keep these semantics and the patch is accepted, you should
> document here what TYPE means.  IIUC any type expression allowed by
> `cl-typep' will be possible, right?  Then just say something like "TYPE
> is a type name like in `cl-typep'" or so.

Thanks, that's a good point.  This new patch improves the docstring and
the changes to the manual, and it adds a test for a cl-typep list form.
Please let me know if more improvements should be made.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern, etc. --]
[-- Type: text/x-diff, Size: 3124 bytes --]

From e588e09a708e23e3d6e5b140c8fde4b0aabd60c3 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/cl-macs.el: Add type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

Co-authored-by: Stefan Monnier <monnier@iro.umontreal.ca>
---
 doc/lispref/control.texi            |  7 +++++++
 etc/NEWS                            |  8 ++++++++
 lisp/emacs-lisp/cl-macs.el          |  7 +++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 32 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index c601e3a..dc91fc1 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,13 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
+
+Example: @code{(type integer)}
+Example: @code{(type (integer 0 10))}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.
diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..c8227c0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,14 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+The new 'type' pattern compares types using 'cl-typep', which allows
+comparing simple types like '(type integer)', as well as forms like
+'(type (integer 0 10))'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 4c2f589..dc272e3 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3403,6 +3403,13 @@ cl-struct-slot-value
 
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..97f3e55 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,16 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-09 18:45 ` Eric Abrahamsen
@ 2020-03-10 15:48   ` Adam Porter
  2020-03-10 17:40     ` Eric Abrahamsen
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2020-03-10 15:48 UTC (permalink / raw)
  To: emacs-devel

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> One thing that's nice about cl-typep is that it will correctly identify
> instances of subclasses, which can be nice. Dunno how difficult it might
> be to work that in there, but it's something to consider. Either way
> this is a nice addition.

Hi Eric,

Please see the latest patch, attached to my reply to Michael.  As
recommended, it uses cl-typep, which allows patterns like:

(type integer)
(type (integer 0 10))

And it should work with subclasses as well.

Thanks.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 15:46               ` Adam Porter
@ 2020-03-10 16:24                 ` Michael Heerdegen
  2020-03-10 17:03                   ` Adam Porter
  2020-03-16 12:59                 ` Stefan Monnier
  1 sibling, 1 reply; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-10 16:24 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> +(pcase-defmacro type (type)
> +  "Pcase pattern that matches objects of TYPE.
> +TYPE is a symbol or list as accepted by `cl-typep', which see."
> +  `(pred (pcase--flip cl-typep ',type)))
> +

Is there a reason why you omit the (require 'cl-lib) call in the
definition as suggested by Stefan?

With your definition in emacs -Q:

(macroexpand-all '(pcase 10 ((type (integer 0 10)) t))) =>
  (if (cl-typep 10 '(integer 0 10)) (progn t) nil)

With Stefan's version:

(macroexpand-all '(pcase 10 ((type (integer 0 10)) t))) =>
  (if (and (integerp 10) (>= 10 '0) (<= 10 '10)) (progn t) nil)

I.e. the type expression is treated at compile time and the dependency
to cl-lib is only at compile time, the compiled code has no dependency.
That should be better, no?


Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 16:24                 ` Michael Heerdegen
@ 2020-03-10 17:03                   ` Adam Porter
  2020-03-10 17:43                     ` Michael Heerdegen
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2020-03-10 17:03 UTC (permalink / raw)
  To: emacs-devel

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Adam Porter <adam@alphapapa.net> writes:
>
>> +(pcase-defmacro type (type)
>> +  "Pcase pattern that matches objects of TYPE.
>> +TYPE is a symbol or list as accepted by `cl-typep', which see."
>> +  `(pred (pcase--flip cl-typep ',type)))
>> +
>
> Is there a reason why you omit the (require 'cl-lib) call in the
> definition as suggested by Stefan?

My understanding was that adding the (require 'cl-lib) form was
suggested if the definition was left in pcase.el, and if the definition
were added to cl-macs.el instead, the (require 'cl-lib) form would not
be needed.  This made sense to me, because cl-macs.el has (require
'cl-lib) at the top of the file.  Of course, I may have misunderstood,
and there are nuances to autoloading and such that I don't fully
understand.

> With your definition in emacs -Q:
>
> (macroexpand-all '(pcase 10 ((type (integer 0 10)) t))) =>
>   (if (cl-typep 10 '(integer 0 10)) (progn t) nil)
>
> With Stefan's version:
>
> (macroexpand-all '(pcase 10 ((type (integer 0 10)) t))) =>
>   (if (and (integerp 10) (>= 10 '0) (<= 10 '10)) (progn t) nil)
>
> I.e. the type expression is treated at compile time and the dependency
> to cl-lib is only at compile time, the compiled code has no dependency.
> That should be better, no?

Of course, that would be better.  However, I was unable to reproduce
those results on my end.  Here are the steps I followed, both on my
main Emacs 26.3 installation and on a build from near master:

1.  Run emacs -Q.
2.  Evaluate this form in *scratch*:

  (pcase-defmacro type (type)
    "Pcase pattern that matches objects of TYPE.
  TYPE is a symbol or list as accepted by `cl-typep', which see."
    `(pred (pcase--flip cl-typep ',type)))

3.  Evaluate this form in *scratch*:

  (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))

Result:

  (if (and (integerp 10) (>= 10 (quote 0)) (<= 10 (quote 10))) (progn t)
  nil)

I haven't done much work on Emacs's core files before, so I may be doing
something wrong.  Please let me know.  :)

However, I have attached another version of the patch which adds an
autoload for the (pcase-defmacro type) form, which seems proper since
the (pcase-defmacro cl-struct) form in the same file also has one.

Thanks for your help and feedback.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add pcase type pattern, etc. --]
[-- Type: text/x-diff, Size: 3141 bytes --]

From 3ab1eae0e71a495bb4e86012ad772146454aa4ff Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/cl-macs.el: Add type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

Co-authored-by: Stefan Monnier <monnier@iro.umontreal.ca>
---
 doc/lispref/control.texi            |  7 +++++++
 etc/NEWS                            |  8 ++++++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index c601e3a..dc91fc1 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,13 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
+
+Example: @code{(type integer)}
+Example: @code{(type (integer 0 10))}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.
diff --git a/etc/NEWS b/etc/NEWS
index 47b87af..c8227c0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,6 +174,14 @@ key             binding
 / v             package-menu-filter-by-version
 / /             package-menu-filter-clear
 
+** pcase.el
+
+*** Added Pcase 'type' pattern.
+
+The new 'type' pattern compares types using 'cl-typep', which allows
+comparing simple types like '(type integer)', as well as forms like
+'(type (integer 0 10))'.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 4c2f589..556ffb3 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3403,6 +3403,14 @@ cl-struct-slot-value
 
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 0b69bd9..97f3e55 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -71,6 +71,16 @@ pcase-tests-member
 (ert-deftest pcase-tests-vectors ()
   (should (equal (pcase [1 2] (`[,x] 1) (`[,x ,y] (+ x y))) 3)))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 ;; Local Variables:
 ;; no-byte-compile: t
 ;; End:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 15:48   ` Adam Porter
@ 2020-03-10 17:40     ` Eric Abrahamsen
  0 siblings, 0 replies; 36+ messages in thread
From: Eric Abrahamsen @ 2020-03-10 17:40 UTC (permalink / raw)
  To: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> One thing that's nice about cl-typep is that it will correctly identify
>> instances of subclasses, which can be nice. Dunno how difficult it might
>> be to work that in there, but it's something to consider. Either way
>> this is a nice addition.
>
> Hi Eric,
>
> Please see the latest patch, attached to my reply to Michael.  As
> recommended, it uses cl-typep, which allows patterns like:
>
> (type integer)
> (type (integer 0 10))
>
> And it should work with subclasses as well.
>
> Thanks.

Works perfectly! Thanks for doing this.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 17:03                   ` Adam Porter
@ 2020-03-10 17:43                     ` Michael Heerdegen
  2020-03-10 18:04                       ` Adam Porter
  0 siblings, 1 reply; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-10 17:43 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> My understanding was that adding the (require 'cl-lib) form was
> suggested if the definition was left in pcase.el, and if the definition
> were added to cl-macs.el instead, the (require 'cl-lib) form would not
> be needed.

Oh, I missed that, then sorry for the noise.

> 1.  Run emacs -Q.
> 2.  Evaluate this form in *scratch*:
>
>   (pcase-defmacro type (type)
>     "Pcase pattern that matches objects of TYPE.
>   TYPE is a symbol or list as accepted by `cl-typep', which see."
>     `(pred (pcase--flip cl-typep ',type)))
>
> 3.  Evaluate this form in *scratch*:
>
>   (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))
>
> Result:
>
>   (if (and (integerp 10) (>= 10 (quote 0)) (<= 10 (quote 10))) (progn t)
>   nil)

No, I don't see that here.  I'm using current master here.

> However, I have attached another version of the patch which adds an
> autoload for the (pcase-defmacro type) form, which seems proper since
> the (pcase-defmacro cl-struct) form in the same file also has one.

What effect does this have?  Since it's no defmacro from, the form is
just copied literally to the generated loaddefs file, right?  Wouldn't
that make a an explicit `require' in the definition necessary?

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 17:43                     ` Michael Heerdegen
@ 2020-03-10 18:04                       ` Adam Porter
  2020-03-10 18:17                         ` Adam Porter
  2020-03-11  2:08                         ` Michael Heerdegen
  0 siblings, 2 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-10 18:04 UTC (permalink / raw)
  To: emacs-devel

Michael Heerdegen <michael_heerdegen@web.de> writes:

>> My understanding was that adding the (require 'cl-lib) form was
>> suggested if the definition was left in pcase.el, and if the definition
>> were added to cl-macs.el instead, the (require 'cl-lib) form would not
>> be needed.
>
> Oh, I missed that, then sorry for the noise.

No problem.  Thanks.

>> 1.  Run emacs -Q.
>> 2.  Evaluate this form in *scratch*:
>>
>>   (pcase-defmacro type (type)
>>     "Pcase pattern that matches objects of TYPE.
>>   TYPE is a symbol or list as accepted by `cl-typep', which see."
>>     `(pred (pcase--flip cl-typep ',type)))
>>
>> 3.  Evaluate this form in *scratch*:
>>
>>   (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))
>>
>> Result:
>>
>>   (if (and (integerp 10) (>= 10 (quote 0)) (<= 10 (quote 10))) (progn t)
>>   nil)
>
> No, I don't see that here.  I'm using current master here.

Hm, I don't know how to explain that.  I guess I'll have to rebuild
master and check again.

>> However, I have attached another version of the patch which adds an
>> autoload for the (pcase-defmacro type) form, which seems proper since
>> the (pcase-defmacro cl-struct) form in the same file also has one.
>
> What effect does this have?  Since it's no defmacro from, the form is
> just copied literally to the generated loaddefs file, right?  Wouldn't
> that make a an explicit `require' in the definition necessary?

The (pcase-defmacro type) form expands to:

  (progn
    (defun type--pcase-macroexpander
        (type)
      ;; [long docstring omitted here]
      `(pred
        (pcase--flip cl-typep ',type)))
    (define-symbol-prop 'type--pcase-macroexpander 'edebug-form-spec 'nil)
    (define-symbol-prop 'type 'pcase-macroexpander #'type--pcase-macroexpander))

My understanding is that the autoload cookie causes the entire
cl-macs.el file to be loaded when the type--pcase-macroexpander function
is called, which then causes cl-lib to be loaded.  So an additional
(require 'cl-lib) would be redundant.  However, as I said, there are
nuances to autoloading that I haven't grokked yet, so I may be missing
something.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 18:04                       ` Adam Porter
@ 2020-03-10 18:17                         ` Adam Porter
  2020-03-10 19:07                           ` Michael Heerdegen
  2020-03-10 19:10                           ` Eric Abrahamsen
  2020-03-11  2:08                         ` Michael Heerdegen
  1 sibling, 2 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-10 18:17 UTC (permalink / raw)
  To: emacs-devel

Hi Michael,

Adam Porter <adam@alphapapa.net> writes:

>>> 1.  Run emacs -Q.
>>> 2.  Evaluate this form in *scratch*:
>>>
>>>   (pcase-defmacro type (type)
>>>     "Pcase pattern that matches objects of TYPE.
>>>   TYPE is a symbol or list as accepted by `cl-typep', which see."
>>>     `(pred (pcase--flip cl-typep ',type)))
>>>
>>> 3.  Evaluate this form in *scratch*:
>>>
>>>   (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))
>>>
>>> Result:
>>>
>>>   (if (and (integerp 10) (>= 10 (quote 0)) (<= 10 (quote 10))) (progn t)
>>>   nil)
>>
>> No, I don't see that here.  I'm using current master here.
>
> Hm, I don't know how to explain that.  I guess I'll have to rebuild
> master and check again.

Here are the steps I just followed:

1.  Pull master and rebase my patch on it.
2.  ./configure && make && src/emacs -Q.
3.  Evaluate this expression, in both *scratch* and M-:, which give the
    same result:

  (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))

Result:

  (if (and (integerp 10) (>= 10 '0) (<= 10 '10)) (progn t) nil)

I can't explain why you get a different result.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 18:17                         ` Adam Porter
@ 2020-03-10 19:07                           ` Michael Heerdegen
  2020-03-10 19:19                             ` Adam Porter
  2020-03-10 19:10                           ` Eric Abrahamsen
  1 sibling, 1 reply; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-10 19:07 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> 1.  Pull master and rebase my patch on it.

I didn't try that.  The last time you said you also evaluated the pcase
macro definition in *scratch*.  That's what I did, in a master built
with your patch not applied.  Anyway, I guess this doesn't matter
much...

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 18:17                         ` Adam Porter
  2020-03-10 19:07                           ` Michael Heerdegen
@ 2020-03-10 19:10                           ` Eric Abrahamsen
  2020-03-10 19:21                             ` Adam Porter
  1 sibling, 1 reply; 36+ messages in thread
From: Eric Abrahamsen @ 2020-03-10 19:10 UTC (permalink / raw)
  To: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> Hi Michael,
>
> Adam Porter <adam@alphapapa.net> writes:
>
>>>> 1.  Run emacs -Q.
>>>> 2.  Evaluate this form in *scratch*:
>>>>
>>>>   (pcase-defmacro type (type)
>>>>     "Pcase pattern that matches objects of TYPE.
>>>>   TYPE is a symbol or list as accepted by `cl-typep', which see."
>>>>     `(pred (pcase--flip cl-typep ',type)))
>>>>
>>>> 3.  Evaluate this form in *scratch*:
>>>>
>>>>   (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))
>>>>
>>>> Result:
>>>>
>>>>   (if (and (integerp 10) (>= 10 (quote 0)) (<= 10 (quote 10))) (progn t)
>>>>   nil)
>>>
>>> No, I don't see that here.  I'm using current master here.
>>
>> Hm, I don't know how to explain that.  I guess I'll have to rebuild
>> master and check again.
>
> Here are the steps I just followed:
>
> 1.  Pull master and rebase my patch on it.
> 2.  ./configure && make && src/emacs -Q.
> 3.  Evaluate this expression, in both *scratch* and M-:, which give the
>     same result:
>
>   (macroexpand-all '(pcase 10 ((type (integer 0 10)) t)))
>
> Result:
>
>   (if (and (integerp 10) (>= 10 '0) (<= 10 '10)) (progn t) nil)
>
> I can't explain why you get a different result.

FWIW, I get the same. `if' when there's only one branch, `cond' when
there's more.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 19:07                           ` Michael Heerdegen
@ 2020-03-10 19:19                             ` Adam Porter
  0 siblings, 0 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-10 19:19 UTC (permalink / raw)
  To: emacs-devel

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Adam Porter <adam@alphapapa.net> writes:
>
>> 1.  Pull master and rebase my patch on it.
>
> I didn't try that.  The last time you said you also evaluated the pcase
> macro definition in *scratch*.  That's what I did, in a master built
> with your patch not applied.  Anyway, I guess this doesn't matter
> much...

Right, I did that as well, and I got the same result as when I rebuilt
Emacs with the patch applied.  I can't explain why you got a different
result.  Well, at least this has been thoroughly tested now.  :)




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 19:10                           ` Eric Abrahamsen
@ 2020-03-10 19:21                             ` Adam Porter
  0 siblings, 0 replies; 36+ messages in thread
From: Adam Porter @ 2020-03-10 19:21 UTC (permalink / raw)
  To: emacs-devel

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> FWIW, I get the same. `if' when there's only one branch, `cond' when
> there's more.

Thanks for testing it.




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 18:04                       ` Adam Porter
  2020-03-10 18:17                         ` Adam Porter
@ 2020-03-11  2:08                         ` Michael Heerdegen
  1 sibling, 0 replies; 36+ messages in thread
From: Michael Heerdegen @ 2020-03-11  2:08 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

Adam Porter <adam@alphapapa.net> writes:

> The (pcase-defmacro type) form expands to:
>
>   (progn
>     (defun type--pcase-macroexpander
>         (type)
>       ;; [long docstring omitted here]
>       `(pred
>         (pcase--flip cl-typep ',type)))
>     (define-symbol-prop 'type--pcase-macroexpander 'edebug-form-spec 'nil)
>     (define-symbol-prop 'type 'pcase-macroexpander #'type--pcase-macroexpander))
>
> My understanding is that the autoload cookie causes the entire
> cl-macs.el file to be loaded when the type--pcase-macroexpander function
> is called, which then causes cl-lib to be loaded.  So an additional
> (require 'cl-lib) would be redundant.  However, as I said, there are
> nuances to autoloading that I haven't grokked yet, so I may be missing
> something.

You understand better than me at least... Ok, `pcase--macroexpand' finds
the expander function by looking at the symbol property, and when
compiling the pattern the expander function is called to expand and
"cl-macs" gets loaded.  That should be alright.

Michael.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add type pattern
  2020-03-10 15:46               ` Adam Porter
  2020-03-10 16:24                 ` Michael Heerdegen
@ 2020-03-16 12:59                 ` Stefan Monnier
  2021-07-11  1:33                   ` [PATCH] pcase.el: Add cl-type and type patterns Adam Porter
  1 sibling, 1 reply; 36+ messages in thread
From: Stefan Monnier @ 2020-03-16 12:59 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> Thanks, that's a good point.  This new patch improves the docstring and
> the changes to the manual, and it adds a test for a cl-typep list form.
> Please let me know if more improvements should be made.

The code looks good, except for the fact that it only works after
(require 'cl-lib), so if we want to put it into `cl-lib` like that, we
should use a name like `cl-type` rather than `type`.


        Stefan




^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] pcase.el: Add cl-type and type patterns
  2020-03-16 12:59                 ` Stefan Monnier
@ 2021-07-11  1:33                   ` Adam Porter
  2021-07-11  2:12                     ` Adam Porter
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2021-07-11  1:33 UTC (permalink / raw)
  To: emacs-devel

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

Hi Stefan, et al,

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

>> Thanks, that's a good point.  This new patch improves the docstring
>> and the changes to the manual, and it adds a test for a cl-typep list
>> form.  Please let me know if more improvements should be made.
>
> The code looks good, except for the fact that it only works after
> (require 'cl-lib), so if we want to put it into `cl-lib` like that, we
> should use a name like `cl-type` rather than `type`.

Revisiting this patch from last year (thread here:
<https://lists.gnu.org/archive/html/emacs-devel/2020-03/msg00411.html>):

I'm attaching two new patches, rebased on current master:

1.  The first implements the new cl-type-based matcher, renamed to
`cl-type' as requested.

2.  The second applies on top of the first and, like the initial patch
proposed in the thread, adds a "dumb" `type' matcher which just uses a
`TYPEp' or `TYPE-p' predicate as appropriate.  My rationale for also
adding this matcher is for writing concise code: with only the cl-type
matcher, the equivalent `pred' matcher is shorter:

  (pred integerp)
  (cl-type integer)

But by also adding `type', for simple types, we can do better, having
the best of clarity and conciseness:

  (type integer)

Since it was decided that the cl-type-based matcher was preferable as an
alternative to the "dumb" matcher (due to cl-type's extra power, and its
correctness in matching structs defined with custom predicate names), I
propose this as a separate patch; if having both is not acceptable, the
second patch may be omitted.

Thanks,
Adam

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cl-type-based pcase matcher --]
[-- Type: text/x-diff, Size: 3263 bytes --]

From d24ca51e670ca8d49e7caae377eeebb528211caf Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH 1/2] * lisp/emacs-lisp/cl-macs.el: Add cl-type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'cl-type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-cl-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

With thanks to Stefan Monnier for his guidance.
---
 doc/lispref/control.texi            |  7 +++++++
 etc/NEWS                            |  5 +++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 30 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 22b665b..efd9394 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,13 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (cl-type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
+
+Example: @code{(cl-type integer)}
+Example: @code{(cl-type (integer 0 10))}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.  The test can be negated with the syntax
diff --git a/etc/NEWS b/etc/NEWS
index 923cfcc..4505c6c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,6 +515,11 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
+*** Added Pcase 'type' pattern.
+The new 'type' pattern compares types using 'cl-typep', which allows
+comparing simple types like '(type integer)', as well as forms like
+'(type (integer 0 10))'.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index cff4368..4cfc07a 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3623,6 +3623,14 @@ cl-struct-slot-value
                         "use `with-eval-after-load' instead." "28.1")
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro cl-type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 2120139..02d3878 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,4 +100,14 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-cl-type ()
+  (should (equal (pcase 1
+                   ((cl-type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((cl-type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((cl-type notatype) 'integer))))
+
 ;;; pcase-tests.el ends here.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: "dumb" type pcase matcher --]
[-- Type: text/x-diff, Size: 3740 bytes --]

From b989a922fa712662cd24363ba4881ee42cd6add4 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sat, 10 Jul 2021 19:46:57 -0500
Subject: [PATCH 2/2] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el ((pcase-defmacro type)): Add 'type'
pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

* etc/NEWS: Update news.

With thanks to Stefan Monnier for his guidance.
---
 doc/lispref/control.texi            |  8 ++++++++
 etc/NEWS                            | 12 ++++++++----
 lisp/emacs-lisp/pcase.el            | 11 +++++++++++
 test/lisp/emacs-lisp/pcase-tests.el |  7 +++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index efd9394..63acd1f 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,14 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, i.e. a type for which a
+corresponding @code{typep} or @code{type-p} predicate is defined.  For
+matching structs defined with @code{cl-defstruct}, the @code{cl-type}
+matcher should be used instead.
+
+Example: @code{(type integer)}
+
 @item (cl-type @var{type})
 Matches if @var{expval} is of type @var{type}, which is a symbol or
 list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
diff --git a/etc/NEWS b/etc/NEWS
index 4505c6c..b25e526 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,10 +515,14 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
-*** Added Pcase 'type' pattern.
-The new 'type' pattern compares types using 'cl-typep', which allows
-comparing simple types like '(type integer)', as well as forms like
-'(type (integer 0 10))'.
+*** Added Pcase 'type' and 'cl-type' patterns.
+
+The new 'type' pattern compares simple types using the corresponding
+'typep' or 'type-p' predicate, e.g. '(type integer)'.
+
+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))'.
 
 +++
 ** profiler.el
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517d..c7e2027 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -1041,5 +1041,16 @@ let
 ;;   (declare (debug (form)))
 ;;   `(pred (lambda (_) ,expr)))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+This checks using a predicate named `TYPEp' or `TYPE-p', as
+appropriate.  For matching structs defined with `cl-defstruct',
+the `cl-type' pattern matcher should be used instead."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 02d3878..9184485 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,6 +100,13 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 (ert-deftest pcase-tests-cl-type ()
   (should (equal (pcase 1
                    ((cl-type integer) 'integer))
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-11  1:33                   ` [PATCH] pcase.el: Add cl-type and type patterns Adam Porter
@ 2021-07-11  2:12                     ` Adam Porter
  2021-07-11  6:11                       ` Eli Zaretskii
  2021-07-16 22:41                       ` Stefan Monnier
  0 siblings, 2 replies; 36+ messages in thread
From: Adam Porter @ 2021-07-11  2:12 UTC (permalink / raw)
  To: emacs-devel

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

Adam Porter <adam@alphapapa.net> writes:

> I'm attaching two new patches, rebased on current master:

It seems inevitable that I discover a mistake after posting a new patch
series.  :)  I corrected a mistake in the NEWS file.  Please see these
new patches.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: cl-type-based pcase matcher --]
[-- Type: text/x-diff, Size: 3275 bytes --]

From a4f030562cf65fda4cf4cbe1c460c880e9f8d914 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH 1/2] * lisp/emacs-lisp/cl-macs.el: Add cl-type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'cl-type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-cl-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

With thanks to Stefan Monnier for his guidance.
---
 doc/lispref/control.texi            |  7 +++++++
 etc/NEWS                            |  5 +++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 30 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 22b665b..efd9394 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,13 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (cl-type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
+
+Example: @code{(cl-type integer)}
+Example: @code{(cl-type (integer 0 10))}
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.  The test can be negated with the syntax
diff --git a/etc/NEWS b/etc/NEWS
index 923cfcc..f2f67a4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,6 +515,11 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
+*** Added Pcase 'cl-type' pattern.
+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))'.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index cff4368..4cfc07a 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3623,6 +3623,14 @@ cl-struct-slot-value
                         "use `with-eval-after-load' instead." "28.1")
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro cl-type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 2120139..02d3878 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,4 +100,14 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-cl-type ()
+  (should (equal (pcase 1
+                   ((cl-type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((cl-type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((cl-type notatype) 'integer))))
+
 ;;; pcase-tests.el ends here.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: "dumb" type pcase matcher --]
[-- Type: text/x-diff, Size: 3752 bytes --]

From 36e6a5880f2548c70a18e0d3266060ec5d49740f Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Sat, 10 Jul 2021 19:46:57 -0500
Subject: [PATCH 2/2] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el ((pcase-defmacro type)): Add 'type'
pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

* etc/NEWS: Update news.

With thanks to Stefan Monnier for his guidance.
---
 doc/lispref/control.texi            |  8 ++++++++
 etc/NEWS                            | 12 ++++++++----
 lisp/emacs-lisp/pcase.el            | 11 +++++++++++
 test/lisp/emacs-lisp/pcase-tests.el |  7 +++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index efd9394..63acd1f 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,14 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, i.e. a type for which a
+corresponding @code{typep} or @code{type-p} predicate is defined.  For
+matching structs defined with @code{cl-defstruct}, the @code{cl-type}
+matcher should be used instead.
+
+Example: @code{(type integer)}
+
 @item (cl-type @var{type})
 Matches if @var{expval} is of type @var{type}, which is a symbol or
 list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
diff --git a/etc/NEWS b/etc/NEWS
index f2f67a4..b25e526 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,10 +515,14 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
-*** Added Pcase 'cl-type' pattern.
-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))'.
+*** Added Pcase 'type' and 'cl-type' patterns.
+
+The new 'type' pattern compares simple types using the corresponding
+'typep' or 'type-p' predicate, e.g. '(type integer)'.
+
+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))'.
 
 +++
 ** profiler.el
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517d..c7e2027 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -1041,5 +1041,16 @@ let
 ;;   (declare (debug (form)))
 ;;   `(pred (lambda (_) ,expr)))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+This checks using a predicate named `TYPEp' or `TYPE-p', as
+appropriate.  For matching structs defined with `cl-defstruct',
+the `cl-type' pattern matcher should be used instead."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 02d3878..9184485 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,6 +100,13 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 (ert-deftest pcase-tests-cl-type ()
   (should (equal (pcase 1
                    ((cl-type integer) 'integer))
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-11  2:12                     ` Adam Porter
@ 2021-07-11  6:11                       ` Eli Zaretskii
  2021-07-16  9:32                         ` Adam Porter
  2021-07-16 22:41                       ` Stefan Monnier
  1 sibling, 1 reply; 36+ messages in thread
From: Eli Zaretskii @ 2021-07-11  6:11 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> From: Adam Porter <adam@alphapapa.net>
> Date: Sat, 10 Jul 2021 21:12:41 -0500
> 
> It seems inevitable that I discover a mistake after posting a new patch
> series.  :)  I corrected a mistake in the NEWS file.  Please see these
> new patches.

A couple of comments about the documentation parts below.  I'll leave
the main part to other reviewers.

> +@item (cl-type @var{type})
> +Matches if @var{expval} is of type @var{type}, which is a symbol or
> +list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.

Please don't use @ref with this "HTML-style", it produces bad results
in Info.  Instead, use a slightly more wordy

  ... as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
  Extensions}).

> +Example: @code{(cl-type integer)}
> +Example: @code{(cl-type (integer 0 10))}

Examples should be in a @lisp..@end lisp block, and then you can drop
the @code markup, which is applied automatically.

> +*** Added Pcase 'cl-type' pattern.
             ^^^^^
"pcase", without capitalization.

Thanks.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-11  6:11                       ` Eli Zaretskii
@ 2021-07-16  9:32                         ` Adam Porter
  2021-07-17 11:58                           ` Eli Zaretskii
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2021-07-16  9:32 UTC (permalink / raw)
  To: emacs-devel

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

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> A couple of comments about the documentation parts below.  I'll leave
> the main part to other reviewers.

>> +@item (cl-type @var{type})
>> +Matches if @var{expval} is of type @var{type}, which is a symbol or
>> +list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
>
> Please don't use @ref with this "HTML-style", it produces bad results
> in Info.  Instead, use a slightly more wordy
>
>   ... as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
>   Extensions}).

Sure, I've changed it.

>> +Example: @code{(cl-type integer)}
>> +Example: @code{(cl-type (integer 0 10))}
>
> Examples should be in a @lisp..@end lisp block, and then you can drop
> the @code markup, which is applied automatically.

Ok, I've changed that two-line example to use "@lisp...@end lisp".  I've
left the one-line example in the second patch in the "Example:
@code{...}" format, which matches all of the other single-line examples
in control.texi.  If you want me to change that one as well, let me
know.

>> +*** Added Pcase 'cl-type' pattern.
>              ^^^^^
> "pcase", without capitalization.

The "Pcase" there was actually redundant, being present in the parent
heading, so I've removed it.

Please see the attached, updated patches.  Thanks for your help.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add cl-typep pattern --]
[-- Type: text/x-diff, Size: 3310 bytes --]

From 2ecda2e27246e3bff2d0cc7c2f89a77c765e6aa5 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH 1/2] * lisp/emacs-lisp/cl-macs.el: Add cl-type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'cl-type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-cl-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

With thanks to Stefan Monnier and Eli Zaretskii for their guidance.
---
 doc/lispref/control.texi            | 10 ++++++++++
 etc/NEWS                            |  5 +++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 22b665b..fc91d6c 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,16 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (cl-type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a symbol or
+list as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
+Extensions}).  Examples:
+
+@lisp
+(cl-type integer)
+(cl-type (integer 0 10))
+@end lisp
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.  The test can be negated with the syntax
diff --git a/etc/NEWS b/etc/NEWS
index 923cfcc..3af842c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,6 +515,11 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
+*** Added 'cl-type' pattern.
+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))'.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index cff4368..4cfc07a 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3623,6 +3623,14 @@ cl-struct-slot-value
                         "use `with-eval-after-load' instead." "28.1")
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro cl-type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a symbol or list as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 2120139..02d3878 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,4 +100,14 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-cl-type ()
+  (should (equal (pcase 1
+                   ((cl-type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((cl-type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((cl-type notatype) 'integer))))
+
 ;;; pcase-tests.el ends here.
-- 
2.7.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: Add type pattern --]
[-- Type: text/x-diff, Size: 3766 bytes --]

From bf8ad829e168fde14f649f1ce167a2a4c9c5e115 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Fri, 16 Jul 2021 04:29:21 -0500
Subject: [PATCH 2/2] * lisp/emacs-lisp/pcase.el: Add type pattern

* lisp/emacs-lisp/pcase.el ((pcase-defmacro type)): Add 'type'
pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

* etc/NEWS: Update news.

With thanks to Stefan Monnier and Eli Zaretskii for their guidance.
---
 doc/lispref/control.texi            |  8 ++++++++
 etc/NEWS                            | 12 ++++++++----
 lisp/emacs-lisp/pcase.el            | 11 +++++++++++
 test/lisp/emacs-lisp/pcase-tests.el |  7 +++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index fc91d6c..137ce33 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,14 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (type @var{type})
+Matches if @var{expval} is of type @var{type}, i.e. a type for which a
+corresponding @code{typep} or @code{type-p} predicate is defined.  For
+matching structs defined with @code{cl-defstruct}, the @code{cl-type}
+matcher should be used instead.
+
+Example: @code{(type integer)}
+
 @item (cl-type @var{type})
 Matches if @var{expval} is of type @var{type}, which is a symbol or
 list as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common Lisp
diff --git a/etc/NEWS b/etc/NEWS
index 3af842c..62659e2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,10 +515,14 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
-*** Added 'cl-type' pattern.
-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))'.
+*** Added 'type' and 'cl-type' patterns.
+
+The new 'type' pattern compares simple types using the corresponding
+'typep' or 'type-p' predicate, e.g. '(type integer)'.
+
+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))'.
 
 +++
 ** profiler.el
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index 006517d..c7e2027 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -1041,5 +1041,16 @@ let
 ;;   (declare (debug (form)))
 ;;   `(pred (lambda (_) ,expr)))
 
+(pcase-defmacro type (type)
+  "Pcase pattern that matches objects of TYPE.
+This checks using a predicate named `TYPEp' or `TYPE-p', as
+appropriate.  For matching structs defined with `cl-defstruct',
+the `cl-type' pattern matcher should be used instead."
+  (let* ((type (symbol-name type))
+         (pred (or (intern-soft (concat type "p"))
+                   (intern-soft (concat type "-p"))
+                   (error "Unknown type: %s" type))))
+    `(pred ,pred)))
+
 (provide 'pcase)
 ;;; pcase.el ends here
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 02d3878..9184485 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,6 +100,13 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-type ()
+  (should (equal (pcase 1
+                   ((type integer) 'integer))
+                 'integer))
+  (should-error (pcase 1
+                  ((type notatype) 'integer))))
+
 (ert-deftest pcase-tests-cl-type ()
   (should (equal (pcase 1
                    ((cl-type integer) 'integer))
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-11  2:12                     ` Adam Porter
  2021-07-11  6:11                       ` Eli Zaretskii
@ 2021-07-16 22:41                       ` Stefan Monnier
  2021-07-17 16:07                         ` Adam Porter
  1 sibling, 1 reply; 36+ messages in thread
From: Stefan Monnier @ 2021-07-16 22:41 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> +@item (cl-type @var{type})
> +Matches if @var{expval} is of type @var{type}, which is a symbol or
> +list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.

Rather than mention "symbol or list", I'd only refer to `cl-typep`,
maybe something like:

    Matches if @var{expval} is of type @var{type}, which is type descriptor
    as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.

The rest of the first patch looks good to me.

> +(pcase-defmacro type (type)
> +  "Pcase pattern that matches objects of TYPE.
> +This checks using a predicate named `TYPEp' or `TYPE-p', as
> +appropriate.  For matching structs defined with `cl-defstruct',
> +the `cl-type' pattern matcher should be used instead."
> +  (let* ((type (symbol-name type))
> +         (pred (or (intern-soft (concat type "p"))
> +                   (intern-soft (concat type "-p"))
> +                   (error "Unknown type: %s" type))))
> +    `(pred ,pred)))

I'm not convinced this second patch is worth the trouble:

    (type FOO)

is not significantly shorter or simpler than

    (pred FOO-p)

but it is inherently brittle because `intern-soft` may
return non-nil even though there's no predicate by that name, and it may
also let you use a "type" which really isn't one such as
`cl--struct-name` or `looking-at`.

`cl-type` provides that functionality in a cleaner and more
reliable way (it still also relies to some extent on similar heuristic
as your code, admittedly, but I've been working to get rid of it).


        Stefan




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-16  9:32                         ` Adam Porter
@ 2021-07-17 11:58                           ` Eli Zaretskii
  0 siblings, 0 replies; 36+ messages in thread
From: Eli Zaretskii @ 2021-07-17 11:58 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> From: Adam Porter <adam@alphapapa.net>
> Date: Fri, 16 Jul 2021 04:32:04 -0500
> 
> Please see the attached, updated patches.  Thanks for your help.

Thanks, I have no further comments to the documentation parts.



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-16 22:41                       ` Stefan Monnier
@ 2021-07-17 16:07                         ` Adam Porter
  2021-07-30 21:24                           ` Stefan Monnier
  0 siblings, 1 reply; 36+ messages in thread
From: Adam Porter @ 2021-07-17 16:07 UTC (permalink / raw)
  To: emacs-devel

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

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

>> +@item (cl-type @var{type})
>> +Matches if @var{expval} is of type @var{type}, which is a symbol or
>> +list as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
>
> Rather than mention "symbol or list", I'd only refer to `cl-typep`,
> maybe something like:
>
>     Matches if @var{expval} is of type @var{type}, which is type descriptor
>     as accepted by @ref{cl-typep,,,cl,Common Lisp Extensions}.
>
> The rest of the first patch looks good to me.

Thanks, I've made this change in the attached patch.  (FYI, Eli
recommends against using the @ref style, see
<https://lists.gnu.org/archive/html/emacs-devel/2021-07/msg00083.html>.)

> I'm not convinced this second patch is worth the trouble:
>
>     (type FOO)
>
> is not significantly shorter or simpler than
>
>     (pred FOO-p)

It's not significantly shorter, but it does make the purpose a bit more
obvious at a glance ("type" vs. "pred").  :)  But anyway...

> but it is inherently brittle because `intern-soft` may
> return non-nil even though there's no predicate by that name, and it may
> also let you use a "type" which really isn't one such as
> `cl--struct-name` or `looking-at`.
>
> `cl-type` provides that functionality in a cleaner and more
> reliable way (it still also relies to some extent on similar heuristic
> as your code, admittedly, but I've been working to get rid of it).

I understand.  Thanks for your work on that.  I've omitted the "type"
patch from this update.

Thanks for all your help.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Add cl-type matcher --]
[-- Type: text/x-diff, Size: 3308 bytes --]

From 0db5d42a2afdb98470434117803f7fb1ef9fb210 Mon Sep 17 00:00:00 2001
From: Adam Porter <adam@alphapapa.net>
Date: Mon, 9 Mar 2020 13:01:32 -0500
Subject: [PATCH] * lisp/emacs-lisp/cl-macs.el: Add cl-type pattern

* lisp/emacs-lisp/cl-macs.el:
((pcase-defmacro type)): Add 'cl-type' pattern.

* test/lisp/emacs-lisp/pcase-tests.el (pcase-tests-cl-type): Add test.

* doc/lispref/control.texi (pcase Macro): Update manual.

With thanks to Stefan Monnier and Eli Zaretskii for their guidance.
---
 doc/lispref/control.texi            | 10 ++++++++++
 etc/NEWS                            |  5 +++++
 lisp/emacs-lisp/cl-macs.el          |  8 ++++++++
 test/lisp/emacs-lisp/pcase-tests.el | 10 ++++++++++
 4 files changed, 33 insertions(+)

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index 22b665b..5026d0a 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -555,6 +555,16 @@ pcase Macro
 Likewise, it makes no sense to bind keyword symbols
 (@pxref{Constant Variables}).
 
+@item (cl-type @var{type})
+Matches if @var{expval} is of type @var{type}, which is a type
+descriptor as accepted by @code{cl-typep} (@pxref{cl-typep,,,cl,Common
+Lisp Extensions}).  Examples:
+
+@lisp
+(cl-type integer)
+(cl-type (integer 0 10))
+@end lisp
+
 @item (pred @var{function})
 Matches if the predicate @var{function} returns non-@code{nil}
 when called on @var{expval}.  The test can be negated with the syntax
diff --git a/etc/NEWS b/etc/NEWS
index 923cfcc..3af842c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -515,6 +515,11 @@ in better code.
 ---
 *** New function 'pcase-compile-patterns' to write other macros.
 
+*** Added 'cl-type' pattern.
+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))'.
+
 +++
 ** profiler.el
 The results displayed by 'profiler-report' now have the usage figures
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index cff4368..caf8bba 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3623,6 +3623,14 @@ cl-struct-slot-value
                         "use `with-eval-after-load' instead." "28.1")
 (run-hooks 'cl-macs-load-hook)
 
+;;; Pcase type pattern.
+
+;;;###autoload
+(pcase-defmacro cl-type (type)
+  "Pcase pattern that matches objects of TYPE.
+TYPE is a type descriptor as accepted by `cl-typep', which see."
+  `(pred (pcase--flip cl-typep ',type)))
+
 ;; Local variables:
 ;; generated-autoload-file: "cl-loaddefs.el"
 ;; End:
diff --git a/test/lisp/emacs-lisp/pcase-tests.el b/test/lisp/emacs-lisp/pcase-tests.el
index 2120139..02d3878 100644
--- a/test/lisp/emacs-lisp/pcase-tests.el
+++ b/test/lisp/emacs-lisp/pcase-tests.el
@@ -100,4 +100,14 @@ pcase-tests-or-vars
     (should (equal (funcall f 'b1) '(4 5 nil nil)))
     (should (equal (funcall f 'b2) '(nil nil 8 9)))))
 
+(ert-deftest pcase-tests-cl-type ()
+  (should (equal (pcase 1
+                   ((cl-type integer) 'integer))
+                 'integer))
+  (should (equal (pcase 1
+                   ((cl-type (integer 0 2)) 'integer-0<=n<=2))
+                 'integer-0<=n<=2))
+  (should-error (pcase 1
+                  ((cl-type notatype) 'integer))))
+
 ;;; pcase-tests.el ends here.
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH] pcase.el: Add cl-type and type patterns
  2021-07-17 16:07                         ` Adam Porter
@ 2021-07-30 21:24                           ` Stefan Monnier
  0 siblings, 0 replies; 36+ messages in thread
From: Stefan Monnier @ 2021-07-30 21:24 UTC (permalink / raw)
  To: Adam Porter; +Cc: emacs-devel

> Thanks, I've made this change in the attached patch.  (FYI, Eli
> recommends against using the @ref style, see
> <https://lists.gnu.org/archive/html/emacs-devel/2021-07/msg00083.html>.)

Thanks, pushed (and sorry it took so long),


        Stefan




^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2021-07-30 21:24 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 18:19 [PATCH] pcase.el: Add type pattern Adam Porter
2020-03-09 18:38 ` Eli Zaretskii
2020-03-09 19:20   ` Adam Porter
2020-03-09 19:35     ` Adam Porter
2020-03-09 18:38 ` Adam Porter
2020-03-09 18:45 ` Eric Abrahamsen
2020-03-10 15:48   ` Adam Porter
2020-03-10 17:40     ` Eric Abrahamsen
2020-03-09 18:53 ` Stefan Monnier
2020-03-09 19:31   ` Adam Porter
2020-03-09 21:00     ` Stefan Monnier
2020-03-09 21:54       ` Adam Porter
2020-03-09 22:22         ` Michael Heerdegen
2020-03-09 22:46           ` Adam Porter
2020-03-10 14:54             ` Michael Heerdegen
2020-03-10 15:01             ` Michael Heerdegen
2020-03-10 15:46               ` Adam Porter
2020-03-10 16:24                 ` Michael Heerdegen
2020-03-10 17:03                   ` Adam Porter
2020-03-10 17:43                     ` Michael Heerdegen
2020-03-10 18:04                       ` Adam Porter
2020-03-10 18:17                         ` Adam Porter
2020-03-10 19:07                           ` Michael Heerdegen
2020-03-10 19:19                             ` Adam Porter
2020-03-10 19:10                           ` Eric Abrahamsen
2020-03-10 19:21                             ` Adam Porter
2020-03-11  2:08                         ` Michael Heerdegen
2020-03-16 12:59                 ` Stefan Monnier
2021-07-11  1:33                   ` [PATCH] pcase.el: Add cl-type and type patterns Adam Porter
2021-07-11  2:12                     ` Adam Porter
2021-07-11  6:11                       ` Eli Zaretskii
2021-07-16  9:32                         ` Adam Porter
2021-07-17 11:58                           ` Eli Zaretskii
2021-07-16 22:41                       ` Stefan Monnier
2021-07-17 16:07                         ` Adam Porter
2021-07-30 21:24                           ` Stefan Monnier

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).