bug#36444: [PATCH] Improved regexp-opt KEEP-ORDER check

[Noam Postavsky]

Mattias Engdegård <mattiase@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 "\\)"))))