Added `participating_captures_len` #908

orlp · 2022-09-24T15:17:56Z

Long overdue, but this is the first step towards adding #824, as greenlit by @BurntSushi.

This new method of Regex returns the number of capture groups that will be filled during a successful match, or None if this can't be statically determined during Regex compile time.

BurntSushi

Thanks!

One particularly unfortunate thing about timing here is that I have a WIP branch that refactors quite a bit of code that this PR also touches. In particular, HirInfo is dropped in favor of a new Properties type, and the Repetition HIR definition is simplified considerably. So pretty much all of the code in this PR will need to be ported over to that branch. The other unfortunate thing is that I have no timeline as to when that branch will get pulled into master. Maybe by the end of the year?

We could get this merged now, but the merge conflicts are going to be brutal enough that it would be easier to just port the patch by hand.

I can probably do the porting for you, but commit authorship won't be preserved.

Otherwise, there are a few changes here that I think need to be made. I'm also not entirely happy with the name participating_captures_len. I'll think about that.

BurntSushi · 2022-09-24T16:41:17Z

regex-syntax/src/hir/mod.rs

+                info.static_capture_count =
+                    exprs.iter().fold(Some(0), |cnt, e| {
+                        Some(cnt? + e.info.static_capture_count?)
+                    });


This should be in the loop above. Otherwise this iterates over the entire concatenation a second time for no good reason as far as I can tell.

BurntSushi · 2022-09-24T16:42:55Z

regex-syntax/src/hir/mod.rs

+                    exprs.iter().map(|e| e.info.static_capture_count);
+                let first = capture_counts.next().unwrap_or(Some(0));
+                info.static_capture_count = capture_counts
+                    .fold(first, |a, b| if a == b { a } else { None });


It looks like all of this logic can be pushed up into the loop as well.

BurntSushi · 2022-09-24T16:43:42Z

regex-syntax/src/hir/mod.rs

@@ -1484,6 +1533,11 @@ struct HirInfo {
    /// If more attributes need to be added, it is OK to increase the size of
    /// this as appropriate.
    bools: u16,
+


This blank line can be dropped.

BurntSushi · 2022-09-24T16:44:08Z

regex-syntax/src/hir/mod.rs

@@ -1484,6 +1533,11 @@ struct HirInfo {
    /// If more attributes need to be added, it is OK to increase the size of
    /// this as appropriate.
    bools: u16,
+
+    /// How many capture groups this HIR expression deterministically fills.


I still think the word here should be "static" and not "deterministic."

BurntSushi · 2022-09-24T16:53:44Z

regex-syntax/src/hir/mod.rs

@@ -1484,6 +1533,11 @@ struct HirInfo {
    /// If more attributes need to be added, it is OK to increase the size of
    /// this as appropriate.
    bools: u16,
+
+    /// How many capture groups this HIR expression deterministically fills.
+    /// If this number could depend on the input (e.g. an Alternation where the


Replace "input" with "haystack" here please. (I'm trying to be better about always using the word "haystack" when referring to the text being searched.)

BurntSushi · 2022-09-24T16:55:16Z

src/re_bytes.rs

+    pub fn participating_captures_len(&self) -> Option<usize> {
+        self.0.participating_captures_len()
+    }
+


Could you back out the public API changes to regex please? I'm fine adding this to regex-syntax, but I'm less sure about it being in the regex API.

BurntSushi · 2022-09-24T16:56:02Z

tests/api.rs

+            "for regex {test_regex}"
+        );
+    }
+}


Could you move these tests to regex-syntax/src/hir/translate.rs? That's where all of the existing "analysis" tests are. It also pushes them closer to where they are implemented.

orlp · 2022-09-24T17:18:32Z

We could get this merged now, but the merge conflicts are going to be brutal enough that it would be easier to just port the patch by hand.

We should probably coordinate a bit then before I make any further changes. I could re-do the patch on the WIP branch myself if it is in a compiling state and the structure is roughly the same. I don't really see the value of putting it into master though if it's going to be replaced soon anyway. Especially since you said this:

Could you back out the public API changes to regex please? I'm fine adding this to regex-syntax, but I'm less sure about it being in the regex API.

If participating_captures_len is not accessible externally from the Regex object itself, there is no point to this PR other than as preparatory work for the inclusion of extract in regex itself. If I convinced you about this, then that's great and I'm happy to help where possible. But if you are still on the fence about extract then adding participating_captures_len to regex-syntax does nothing for making extract possible as an extension trait.

BurntSushi · 2022-09-24T17:27:22Z

I could re-do the patch on the WIP branch myself if it is in a compiling state and the structure is roughly the same.

It is and the overall structure is indeed the same.

I don't really see the value of putting it into master though if it's going to be replaced soon anyway.

Aye. Just to set expectations here, this branch is the culmination of #656, which I have been working on for years. While it might get merged by the end of this year, I can't make any promises.

If participating_captures_len is not accessible externally from the Regex object itself, there is no point to this PR other than as preparatory work for the inclusion of extract in regex itself. If I convinced you about this, then that's great and I'm happy to help where possible. But if you are still on the fence about extract then adding participating_captures_len to regex-syntax does nothing for making extract possible as an extension trait.

Ah hmmm, yes, I see. You could still do it in another crate, but it would require a lot more ceremony. Namely, a wrapper type around Regex that parsed the pattern a second time to figure out the participating_captures_len property. That's not so good.

OK, I'm fine with adding this to the regex API, but it should come with at least some doc examples. One example might even be the extract implementation itself, since it's so simple.

Thinking about alternative names too... Ideas:

fixed_captures_len
static_captures_len

Other ideas?

orlp · 2022-09-24T18:26:39Z

For the bikeshedding, other options might be unconditional_captures_len, exact_captures_len, guaranteed_captures_len, proven_captures_len. If participating_captures_len is not acceptable I think out of all the proposed ones (including my own) I like static_captures_len the best. But perhaps you like one of my other suggestions better.

Aye. Just to set expectations here, this branch is the culmination of #656, which I have been working on for years. While it might get merged by the end of this year, I can't make any promises.
...
OK, I'm fine with adding this to the regex API, but it should come with at least some doc examples. One example might even be the extract implementation itself, since it's so simple.

Alright then, since you're okay with it going public on Regex I think I will (for now) simply make your requested changes for the master branch, so that hopefully in the next release of regex it's possible to write the checked extension trait and have it be immediately useful.

OK, I'm fine with adding this to the regex API, but it should come with at least some doc examples. One example might even be the extract implementation itself, since it's so simple.

I think while simple it might be a bit too big for an example. I'll make sure to add some examples though.

BurntSushi · 2022-09-24T18:31:20Z

Alright then, since you're okay with it going public on Regex I think I will (for now) simply make your requested changes for the master branch, so that hopefully in the next release of regex it's possible to write the checked extension trait and have it be immediately useful.

Sorry, to be clear, I do not really want to merge this into master. As I said above, the merge conflicts against my WIP branch will be brutal.

orlp · 2022-09-24T18:51:39Z

Alright then, since you're okay with it going public on Regex I think I will (for now) simply make your requested changes for the master branch, so that hopefully in the next release of regex it's possible to write the checked extension trait and have it be immediately useful.

Sorry, to be clear, I do not really want to merge this into master. As I said above, the merge conflicts against my WIP branch will be brutal.

Ah alright. I'll work on a pull request on the other branch then.

BurntSushi · 2023-03-04T18:40:34Z

OK, I've ported this patch to the new branch. And I've put it in the regex API as well. Here is that method and its docs:

    /// Returns the total number of explicit capturing groups that appear in
    /// every possible match.
    ///
    /// If the number of capture groups can vary depending on the match, then
    /// this returns `None`. That is, a value is only returned when the number
    /// of matching groups is invariant or "static."
    ///
    /// Note that like [`Regex::captures_len`], this **does** include the
    /// implicit capturing group corresponding to the entire match. Therefore,
    /// when a non-None value is returned, it is guaranteed to be at least `1`.
    /// Stated differently, a return value of `Some(0)` is impossible.
    ///
    /// # Example
    ///
    /// This shows a few cases where a static number of capture groups is
    /// available and a few cases where it is not.
    ///
    /// ```
    /// use regex::Regex;
    ///
    /// let len = |pattern| {
    ///     Regex::new(pattern).map(|re| re.static_captures_len())
    /// };
    ///
    /// assert_eq!(Some(1), len("a")?);
    /// assert_eq!(Some(2), len("(a)")?);
    /// assert_eq!(Some(2), len("(a)|(b)")?);
    /// assert_eq!(Some(3), len("(a)(b)|(c)(d)")?);
    /// assert_eq!(None, len("(a)|b")?);
    /// assert_eq!(None, len("a|(b)")?);
    /// assert_eq!(None, len("(b)*")?);
    /// assert_eq!(Some(2), len("(b)+")?);
    ///
    /// # Ok::<(), Box<dyn std::error::Error>>(())
    /// ```
    #[inline]
    pub fn static_captures_len(&self) -> Option<usize> { ... }

In particular, there is one crucial difference here (other than the name): for a Some value, the length is always at least 1. This makes it consistent with the value returned by captures_len.

This adds a new routine for computing the static number of capture groups that will appear in every match. If the number of groups is not invariant across all matches, then there is no static capture length. This is meant to help implement higher level convenience APIs for extracting capture groups, such as the one described in #824. We may wind up including such APIs in the regex crate itself, but this commit stops short of that. Instead, we just add this new property which should permit those APIs to exist outside of this crate for now. Closes #908

orlp · 2023-03-04T20:44:58Z

Seems fair enough to me 👍

BurntSushi · 2023-03-04T20:49:19Z

@orlp Also, thank you for the great tests! I was able to copy them over pretty easily, and it looks like they have good coverage. Overall it was pretty easy to port over. The logic was all roughly the same!

This adds a new routine for computing the static number of capture groups that will appear in every match. If the number of groups is not invariant across all matches, then there is no static capture length. This is meant to help implement higher level convenience APIs for extracting capture groups, such as the one described in #824. We may wind up including such APIs in the regex crate itself, but this commit stops short of that. Instead, we just add this new property which should permit those APIs to exist outside of this crate for now. Closes #908

1.8.0 (2023-04-20) ================== This is a sizeable release that will be soon followed by another sizeable release. Both of them will combined close over 40 existing issues and PRs. This first release, despite its size, essentially represent preparatory work for the second release, which will be even bigger. Namely, this release: * Increases the MSRV to Rust 1.60.0, which was released about 1 year ago. * Upgrades its dependency on `aho-corasick` to the recently release 1.0 version. * Upgrades its dependency on `regex-syntax` to the simultaneously released `0.7` version. The changes to `regex-syntax` principally revolve around a rewrite of its literal extraction code and a number of simplifications and optimizations to its high-level intermediate representation (HIR). The second release, which will follow ~shortly after the release above, will contain a soup-to-nuts rewrite of every regex engine. This will be done by bringing [`regex-automata`](https://github.com/BurntSushi/regex-automata) into this repository, and then changing the `regex` crate to be nothing but an API shim layer on top of `regex-automata`'s API. These tandem releases are the culmination of about 3 years of on-and-off work that [began in earnest in March 2020](#656). Because of the scale of changes involved in these releases, I would love to hear about your experience. Especially if you notice undocumented changes in behavior or performance changes (positive *or* negative). Most changes in the first release are listed below. For more details, please see the commit log, which reflects a linear and decently documented history of all changes. New features: * [FEATURE #501](#501): Permit many more characters to be escaped, even if they have no significance. More specifically, any ASCII character except for `[0-9A-Za-z<>]` can now be escaped. Also, a new routine, `is_escapeable_character`, has been added to `regex-syntax` to query whether a character is escapeable or not. * [FEATURE #547](#547): Add `Regex::captures_at`. This filles a hole in the API, but doesn't otherwise introduce any new expressive power. * [FEATURE #595](#595): Capture group names are now Unicode-aware. They can now begin with either a `_` or any "alphabetic" codepoint. After the first codepoint, subsequent codepoints can be any sequence of alpha-numeric codepoints, along with `_`, `.`, `[` and `]`. Note that replacement syntax has not changed. * [FEATURE #810](#810): Add `Match::is_empty` and `Match::len` APIs. * [FEATURE #905](#905): Add an `impl Default for RegexSet`, with the default being the empty set. * [FEATURE #908](#908): A new method, `Regex::static_captures_len`, has been added which returns the number of capture groups in the pattern if and only if every possible match always contains the same number of matching groups. * [FEATURE #955](#955): Named captures can now be written as `(?<name>re)` in addition to `(?P<name>re)`. * FEATURE: `regex-syntax` now supports empty character classes. * FEATURE: `regex-syntax` now has an optional `std` feature. (This will come to `regex` in the second release.) * FEATURE: The `Hir` type in `regex-syntax` has had a number of simplifications made to it. * FEATURE: `regex-syntax` has support for a new `R` flag for enabling CRLF mode. This will be supported in `regex` proper in the second release. * FEATURE: `regex-syntax` now has proper support for "regex that never matches" via `Hir::fail()`. * FEATURE: The `hir::literal` module of `regex-syntax` has been completely re-worked. It now has more documentation, examples and advice. * FEATURE: The `allow_invalid_utf8` option in `regex-syntax` has been renamed to `utf8`, and the meaning of the boolean has been flipped. Performance improvements: * PERF: The upgrade to `aho-corasick 1.0` may improve performance in some cases. It's difficult to characterize exactly which patterns this might impact, but if there are a small number of longish (>= 4 bytes) prefix literals, then it might be faster than before. Bug fixes: * [BUG #514](#514): Improve `Debug` impl for `Match` so that it doesn't show the entire haystack. * BUGS [#516](#516), [#731](#731): Fix a number of issues with printing `Hir` values as regex patterns. * [BUG #610](#610): Add explicit example of `foo|bar` in the regex syntax docs. * [BUG #625](#625): Clarify that `SetMatches::len` does not (regretably) refer to the number of matches in the set. * [BUG #660](#660): Clarify "verbose mode" in regex syntax documentation. * BUG [#738](#738), [#950](#950): Fix `CaptureLocations::get` so that it never panics. * [BUG #747](#747): Clarify documentation for `Regex::shortest_match`. * [BUG #835](#835): Fix `\p{Sc}` so that it is equivalent to `\p{Currency_Symbol}`. * [BUG #846](#846): Add more clarifying documentation to the `CompiledTooBig` error variant. * [BUG #854](#854): Clarify that `regex::Regex` searches as if the haystack is a sequence of Unicode scalar values. * [BUG #884](#884): Replace `__Nonexhaustive` variants with `#[non_exhaustive]` attribute. * [BUG #893](#893): Optimize case folding since it can get quite slow in some pathological cases. * [BUG #895](#895): Reject `(?-u:\W)` in `regex::Regex` APIs. * [BUG #942](#942): Add a missing `void` keyword to indicate "no parameters" in C API. * [BUG #965](#965): Fix `\p{Lc}` so that it is equivalent to `\p{Cased_Letter}`. * [BUG #975](#975): Clarify documentation for `\pX` syntax.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [regex](https://github.com/rust-lang/regex) | dependencies | minor | `1.7.3` -> `1.8.1` | --- ### Release Notes <details> <summary>rust-lang/regex</summary> ### [`v1.8.1`](https://github.com/rust-lang/regex/blob/HEAD/CHANGELOG.md#181-2023-04-21) \================== This is a patch release that fixes a bug where a regex match could be reported where none was found. Specifically, the bug occurs when a pattern contains some literal prefixes that could be extracted *and* an optional word boundary in the prefix. Bug fixes: - [BUG #981](rust-lang/regex#981): Fix a bug where a word boundary could interact with prefix literal optimizations and lead to a false positive match. ### [`v1.8.0`](https://github.com/rust-lang/regex/blob/HEAD/CHANGELOG.md#180-2023-04-20) \================== This is a sizeable release that will be soon followed by another sizeable release. Both of them will combined close over 40 existing issues and PRs. This first release, despite its size, essentially represents preparatory work for the second release, which will be even bigger. Namely, this release: - Increases the MSRV to Rust 1.60.0, which was released about 1 year ago. - Upgrades its dependency on `aho-corasick` to the recently released 1.0 version. - Upgrades its dependency on `regex-syntax` to the simultaneously released `0.7` version. The changes to `regex-syntax` principally revolve around a rewrite of its literal extraction code and a number of simplifications and optimizations to its high-level intermediate representation (HIR). The second release, which will follow ~shortly after the release above, will contain a soup-to-nuts rewrite of every regex engine. This will be done by bringing [`regex-automata`](https://github.com/BurntSushi/regex-automata) into this repository, and then changing the `regex` crate to be nothing but an API shim layer on top of `regex-automata`'s API. These tandem releases are the culmination of about 3 years of on-and-off work that [began in earnest in March 2020](rust-lang/regex#656). Because of the scale of changes involved in these releases, I would love to hear about your experience. Especially if you notice undocumented changes in behavior or performance changes (positive *or* negative). Most changes in the first release are listed below. For more details, please see the commit log, which reflects a linear and decently documented history of all changes. New features: - [FEATURE #501](rust-lang/regex#501): Permit many more characters to be escaped, even if they have no significance. More specifically, any ASCII character except for `[0-9A-Za-z<>]` can now be escaped. Also, a new routine, `is_escapeable_character`, has been added to `regex-syntax` to query whether a character is escapeable or not. - [FEATURE #547](rust-lang/regex#547): Add `Regex::captures_at`. This filles a hole in the API, but doesn't otherwise introduce any new expressive power. - [FEATURE #595](rust-lang/regex#595): Capture group names are now Unicode-aware. They can now begin with either a `_` or any "alphabetic" codepoint. After the first codepoint, subsequent codepoints can be any sequence of alpha-numeric codepoints, along with `_`, `.`, `[` and `]`. Note that replacement syntax has not changed. - [FEATURE #810](rust-lang/regex#810): Add `Match::is_empty` and `Match::len` APIs. - [FEATURE #905](rust-lang/regex#905): Add an `impl Default for RegexSet`, with the default being the empty set. - [FEATURE #908](rust-lang/regex#908): A new method, `Regex::static_captures_len`, has been added which returns the number of capture groups in the pattern if and only if every possible match always contains the same number of matching groups. - [FEATURE #955](rust-lang/regex#955): Named captures can now be written as `(?<name>re)` in addition to `(?P<name>re)`. - FEATURE: `regex-syntax` now supports empty character classes. - FEATURE: `regex-syntax` now has an optional `std` feature. (This will come to `regex` in the second release.) - FEATURE: The `Hir` type in `regex-syntax` has had a number of simplifications made to it. - FEATURE: `regex-syntax` has support for a new `R` flag for enabling CRLF mode. This will be supported in `regex` proper in the second release. - FEATURE: `regex-syntax` now has proper support for "regex that never matches" via `Hir::fail()`. - FEATURE: The `hir::literal` module of `regex-syntax` has been completely re-worked. It now has more documentation, examples and advice. - FEATURE: The `allow_invalid_utf8` option in `regex-syntax` has been renamed to `utf8`, and the meaning of the boolean has been flipped. Performance improvements: - PERF: The upgrade to `aho-corasick 1.0` may improve performance in some cases. It's difficult to characterize exactly which patterns this might impact, but if there are a small number of longish (>= 4 bytes) prefix literals, then it might be faster than before. Bug fixes: - [BUG #514](rust-lang/regex#514): Improve `Debug` impl for `Match` so that it doesn't show the entire haystack. - BUGS [#516](rust-lang/regex#516), [#731](rust-lang/regex#731): Fix a number of issues with printing `Hir` values as regex patterns. - [BUG #610](rust-lang/regex#610): Add explicit example of `foo|bar` in the regex syntax docs. - [BUG #625](rust-lang/regex#625): Clarify that `SetMatches::len` does not (regretably) refer to the number of matches in the set. - [BUG #660](rust-lang/regex#660): Clarify "verbose mode" in regex syntax documentation. - BUG [#738](rust-lang/regex#738), [#950](rust-lang/regex#950): Fix `CaptureLocations::get` so that it never panics. - [BUG #747](rust-lang/regex#747): Clarify documentation for `Regex::shortest_match`. - [BUG #835](rust-lang/regex#835): Fix `\p{Sc}` so that it is equivalent to `\p{Currency_Symbol}`. - [BUG #846](rust-lang/regex#846): Add more clarifying documentation to the `CompiledTooBig` error variant. - [BUG #854](rust-lang/regex#854): Clarify that `regex::Regex` searches as if the haystack is a sequence of Unicode scalar values. - [BUG #884](rust-lang/regex#884): Replace `__Nonexhaustive` variants with `#[non_exhaustive]` attribute. - [BUG #893](rust-lang/regex#893): Optimize case folding since it can get quite slow in some pathological cases. - [BUG #895](rust-lang/regex#895): Reject `(?-u:\W)` in `regex::Regex` APIs. - [BUG #942](rust-lang/regex#942): Add a missing `void` keyword to indicate "no parameters" in C API. - [BUG #965](rust-lang/regex#965): Fix `\p{Lc}` so that it is equivalent to `\p{Cased_Letter}`. - [BUG #975](rust-lang/regex#975): Clarify documentation for `\pX` syntax. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <[email protected]> Co-authored-by: crapStone <[email protected]> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1874 Reviewed-by: crapStone <[email protected]> Co-authored-by: Calciumdibromid Bot <[email protected]> Co-committed-by: Calciumdibromid Bot <[email protected]>

orlp added 3 commits September 24, 2022 16:02

Fixed typo in src/prog.rs.

31cdd44

Added Regex::participating_captures_len.

7874e12

Removed unnecessary unicode-reliant test and ran fmt.

559606c

BurntSushi requested changes Sep 24, 2022

View reviewed changes

BurntSushi added the fix-incoming label Mar 4, 2023

BurntSushi closed this in 4fbf3ea Apr 17, 2023

BurntSushi mentioned this pull request Apr 20, 2023

release: 1.8.0 #979

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `participating_captures_len` #908

Added `participating_captures_len` #908

orlp commented Sep 24, 2022

BurntSushi left a comment

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

BurntSushi Sep 24, 2022

orlp commented Sep 24, 2022 •

edited

Loading

BurntSushi commented Sep 24, 2022

orlp commented Sep 24, 2022 •

edited

Loading

BurntSushi commented Sep 24, 2022

orlp commented Sep 24, 2022

BurntSushi commented Mar 4, 2023

orlp commented Mar 4, 2023

BurntSushi commented Mar 4, 2023

Added participating_captures_len #908

Added participating_captures_len #908

Conversation

orlp commented Sep 24, 2022

BurntSushi left a comment

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

BurntSushi Sep 24, 2022

Choose a reason for hiding this comment

orlp commented Sep 24, 2022 • edited Loading

BurntSushi commented Sep 24, 2022

orlp commented Sep 24, 2022 • edited Loading

BurntSushi commented Sep 24, 2022

orlp commented Sep 24, 2022

BurntSushi commented Mar 4, 2023

orlp commented Mar 4, 2023

BurntSushi commented Mar 4, 2023

Added `participating_captures_len` #908

Added `participating_captures_len` #908

orlp commented Sep 24, 2022 •

edited

Loading

orlp commented Sep 24, 2022 •

edited

Loading