Fix auto-atomicity for loops around required one/notone/set loops #56735
Conversation
Given an expression like `(?:a{2,3}){2}`, we currently erroneously convert that to `(?>a{2,3}){2}`, i.e. making the inner loop atomic, even though that then causes this to fail to match `aaaa` when it should match it (it fails because the first iteration will atomically match `aaa` and thus won't given any back, and then the next iteration will fail to match at least two `a`s).
The simple fix is in FindLastExpressionInLoopForAutoAtomic, removing the special-case that tries to make the body of a loop that's a one/notone/set loop atomic. There are likely special-cases of this special-case that are still valid, but I'm not convinced it's worth trying to maintain and risk other gaps.
|
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue DetailsGiven an expression like The simple fix is in FindLastExpressionInLoopForAutoAtomic, removing the special-case that tries to make the body of a loop that's a one/notone/set loop atomic. There are likely special-cases of this special-case that are still valid, but I'm not convinced it's worth trying to maintain and risk other gaps. Fixes #56721
|
|
Just FYI, I usually debug your Regex PRs locally so I can see what is going on. Don't have time until at least tomorrow to try out this PR. Will review this PR Friday or early next week |
|
Thanks. |
ghost
locked as resolved and limited conversation to collaborators
Given an expression like
(?:a{2,3}){2}, we currently erroneously convert that to(?>a{2,3}){2}, i.e. making the inner loop atomic, even though that then causes this to fail to matchaaaawhen it should match it (it fails because the first iteration will atomically matchaaaand thus won't given any back, and then the next iteration will fail to match at least twoas).The simple fix is in FindLastExpressionInLoopForAutoAtomic, removing the special-case that tries to make the body of a loop that's a one/notone/set loop atomic. There are likely special-cases of this special-case that are still valid, but I'm not convinced it's worth trying to maintain and risk other gaps.
Fixes #56721
cc: @dotnet/area-system-text-regularexpressions, @anreton