emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

master 70f2d65: Fix pcase rx pattern bugs


From: Mattias Engdegård
Subject: master 70f2d65: Fix pcase rx pattern bugs
Date: Fri, 26 Feb 2021 04:28:44 -0500 (EST)

branch: master
commit 70f2d658e42120c289c4a3c043b5d5b1331bc183
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    Fix pcase rx pattern bugs
    
    Two unrelated bugs: A missing type check caused an error in rx
    patterns for non-string match targets, and rx patterns did not work at
    all in pcase-let or pcase-let*.
    
    Second bug reported by Basil Contovounesios and Ag Ibragimov; fixes
    proposed by Stefan Monnier.  Discussion and explanation in thread at
    https://lists.gnu.org/archive/html/emacs-devel/2021-02/msg01924.html
    
    * lisp/emacs-lisp/rx.el (rx): Add (pred stringp) to avoid type errors,
    and replace the `pred` clause for the actual match with something that
    works with pcase-let(*) without being optimised away.
    * test/lisp/emacs-lisp/rx-tests.el (rx-pcase): Add test cases.
---
 lisp/emacs-lisp/rx.el            |  6 +++++-
 test/lisp/emacs-lisp/rx-tests.el | 12 +++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/rx.el b/lisp/emacs-lisp/rx.el
index 58584f3..ffc2195 100644
--- a/lisp/emacs-lisp/rx.el
+++ b/lisp/emacs-lisp/rx.el
@@ -1437,7 +1437,11 @@ following constructs:
                    construct."
   (let* ((rx--pcase-vars nil)
          (regexp (rx--to-expr (rx--pcase-transform (cons 'seq regexps)))))
-    `(and (pred (string-match ,regexp))
+    `(and (pred stringp)
+          ;; `pcase-let' takes a match for granted and discards all unnecessary
+          ;; conditions, which means that a `pred' clause cannot be used for
+          ;; the match condition.  The following construct seems to survive.
+          (app (lambda (s) (string-match ,regexp s)) (pred identity))
           ,@(let ((i 0))
               (mapcar (lambda (name)
                         (setq i (1+ i))
diff --git a/test/lisp/emacs-lisp/rx-tests.el b/test/lisp/emacs-lisp/rx-tests.el
index 12bf4f7..fecdcf5 100644
--- a/test/lisp/emacs-lisp/rx-tests.el
+++ b/test/lisp/emacs-lisp/rx-tests.el
@@ -171,7 +171,17 @@
   (should (equal (pcase "abc"
                    ((rx (? (let x alpha)) (?? (let y alnum)) ?c)
                     (list x y)))
-                 '("a" "b"))))
+                 '("a" "b")))
+  (should (equal (pcase 'not-a-string
+                   ((rx nonl) 'wrong)
+                   (_ 'correct))
+                 'correct))
+  (should (equal (pcase-let (((rx ?B (let z nonl)) "ABC"))
+                   (list 'ok z))
+                 '(ok "C")))
+  (should (equal (pcase-let* (((rx ?E (let z nonl)) "DEF"))
+                   (list 'ok z))
+                 '(ok "F"))))
 
 (ert-deftest rx-kleene ()
   "Test greedy and non-greedy repetition operators."



reply via email to

[Prev in Thread] Current Thread [Next in Thread]