RFR: 8312976: MatchResult produces StringIndexOutOfBoundsException for groups outside match [v4]

[Stuart Marks]

On Mon, 31 Jul 2023 18:38:30 GMT, Raffaello Giulietti <rgiulietti at 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