GNU bug report logs - #36444
[PATCH] Improved regexp-opt KEEP-ORDER check

Previous Next

Package: emacs;

Reported by: Mattias Engdegård <mattiase <at> acm.org>

Date: Sun, 30 Jun 2019 12:30:02 UTC

Severity: normal

Tags: patch

Done: Mattias Engdegård <mattiase <at> acm.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Noam Postavsky <npostavs <at> gmail.com>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: 36444 <at> debbugs.gnu.org
Subject: bug#36444: [PATCH] Improved regexp-opt KEEP-ORDER check
Date: Wed, 03 Jul 2019 15:29:33 -0400
Mattias Engdegård <mattiase <at> acm.org> writes:

> +             ;; The algorithm will generate a pattern that matches
> +             ;; longer strings in the list before shorter.  If the
> +             ;; list order matters, then no string must come after a
> +             ;; proper prefix of that string.  To check this, verify
> +             ;; that a straight or-pattern matches each string
> +             ;; entirely.
> +             ((and keep-order
> +                   (let* ((case-fold-search nil)
> +                          (alts (mapconcat #'regexp-quote strings "\\|")))
> +                     (and (save-match-data

You don't actually need this save-match-data, right?  Because there is
already one at the top level of the function (which I'm also not sure is
really needed, but probably best not to touch that).

> +                            (let ((s strings))
> +                              (while (and s
> +                                          (string-match alts (car s))
> +                                          (= (match-end 0) (length (car s))))
> +                                (setq s (cdr s)))
> +                              s))
> +                          (concat (or open "\\(?:") alts "\\)")))))

IMO, a dolist + catch & throw would be a bit more readable; it took me
some puzzling to realize that the early exit was the "non-optimized"
case.

  (and keep-order
       (let* ((case-fold-search nil)
              (alts (mapconcat #'regexp-quote strings "\\|")))
         (and (catch 'has-prefix
                (dolist (s strings)
                  (unless (and (string-match alts s)
                               (= (match-end 0) (length s)))
                    (throw 'has-prefix s))))
              (concat (or open "\\(?:") alts "\\)"))))




This bug report was last modified 6 years and 18 days ago.

Previous Next


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