Skip to content

Fix #3901: Do not fill groupStartMap for non-matching groups. #3907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2019

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Dec 18, 2019

groupStartMap is initially filled with -1 everywhere, which is the correct value for non-matching groups. When propagating the start and end of a GroupNode, we previously always overrode that value with start, which can >= 0 even for non-matching groups if they are within a negative look-ahead. This is incorrect, as non-matching groups must always report -1 as their start.

We now explicitly avoid overriding the initial -1 for non-matching groups. This more closely corresponds to what was done in GroupStartMap.setEndReturnStart and setStartReturnEnd before the big rewrite of 56dd7ab.

@sjrd sjrd requested a review from gzm0 December 18, 2019 12:21
`groupStartMap` is initially filled with `-1` everywhere, which is
the correct value for non-matching groups. When propagating the
`start` and `end` of a `GroupNode`, we previously always overrode
that value with `start`, which can `>= 0` even for non-matching
groups if they are within a negative look-ahead. This is incorrect,
as non-matching groups must always report `-1` as their `start`.

We now explicitly avoid overriding the initial `-1` for
non-matching groups. This more closely corresponds to what was done
in `GroupStartMap.setEndReturnStart` and `setStartReturnEnd` before
the big rewrite of 56dd7ab.
@sjrd sjrd force-pushed the fix-regex-start-end-regression-3901 branch from 272fb91 to 602005d Compare December 19, 2019 15:50
@sjrd sjrd merged commit 75d8244 into scala-js:0.6.x Dec 20, 2019
@sjrd sjrd deleted the fix-regex-start-end-regression-3901 branch December 20, 2019 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants