On Mon, 31 Jul 2023 18:38:30 GMT, Raffaello Giulietti <rgiulie...@openjdk.org> wrote:
>> Fixes a bug showing up after >> [JDK-8132995](https://bugs.openjdk.org/browse/JDK-8132995) when there are >> capturing groups outside the match. > > Raffaello Giulietti has updated the pull request with a new target base due > to a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains six additional > commits since the last revision: > > - Merged with mainline. > - Merge branch 'master' into 8312976 > - Refactored recent tests as JUnit parametrized test. > - Added a test with negatvie lookbehind. > - Added test with a lookahead containing a back reference. > - 8312976: MatchResult produces StringIndexOutOfBoundsException for groups > outside match Marked as reviewed by smarks (Reviewer). OK, changes and tests look good. Who knew, a capturing group could have bounds outside the matched string! I did notice something that we might want to look at in the future. `MatchResult` has `first` and `last` fields; these seem redundant with the `group[0]` and `group[1]` entries. Indeed, `Matcher` also has `first` and `last` and the `group[]` array; `start()` returns `first` and `end()` returns `last`; while `start(0)` returns `group[0]` and `end(0)` returns `group[1]`. But the javadocs say that `start()` and `start(0)` are equivalent and that `end()` and `end(0)` are also equivalent. So `Matcher` also appears to have this redundancy. Since these values are all set by the matching engine in `Pattern` it seems like the redundancy is spread throughout the code. Conceptually it might be possible for `MatchResult` to omit `first` and `last` and just use the first two `group` array entries, but it's really hard to verify this. Unclear whether this is worthwhile (and it's certainly not worth trying to include in this bugfix). ------------- PR Review: https://git.openjdk.org/jdk/pull/15053#pullrequestreview-1561805692 PR Comment: https://git.openjdk.org/jdk/pull/15053#issuecomment-1664600847
