Fix Regex if-then-else code generation #63427
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue DetailsCode coverage revealed some gaps in testing around conditionals, which in turn led to discovering a) some bugs in both RegexCompiler and the source generator around expression conditionals, and b) divergence between the compiler and the source generator. This adds tests to address the code coverage gaps and fixes the implementations to both address the bugs and bring the code much closer. The main problem with expressional conditionals was flawed handling of backtracking, either not pushing the right state on to the stack or not initializing the state correctly in the first place.
|
ghost
assigned
...libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs
Outdated
Show resolved
Hide resolved
...libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs
Outdated
Show resolved
Hide resolved
|
Seems reasonable to me but probably best @joperezr reviews it more carefully. |
src/libraries/System.Text.RegularExpressions/tests/Regex.Match.Tests.cs
Outdated
Show resolved
Hide resolved
Code coverage revealed some gaps in testing around conditionals, which in turn led to discovering a) some bugs in both RegexCompiler and the source generator around expression conditionals, and b) divergence between the compiler and the source generator. This adds tests to address the code coverage gaps and fixes the implementations to both address the bugs and bring the code much closer. The main problem with expressional conditionals was flawed handling of backtracking, either not pushing the right state on to the stack or not initializing the state correctly in the first place. We also failed to revert positional changes made in the expression condition when the condition failed to match.
stephentoub
force-pushed
the
fixregexconditionals
branch
from
bae0a18 to
d62c182
Compare
|
@danmoseley, I pushed changes that ensure every conditional has both a yes and a no branch, so that every engine will consistently see both and we'll minimize potential divergence. Please take another look. Thanks. |
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/tests/RegexReductionTests.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/tests/Regex.Groups.Tests.cs
Show resolved
Hide resolved
...libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexCompiler.cs
Show resolved
Hide resolved
|
|
|
macOS failures are #63561 |
2 participants
Code coverage revealed some gaps in testing around conditionals, which in turn led to discovering a) some bugs in both RegexCompiler and the source generator around expression conditionals, and b) divergence between the compiler and the source generator. This adds tests to address the code coverage gaps and fixes the implementations to both address the bugs and bring the code much closer. The main problem with expressional conditionals was flawed handling of backtracking, either not pushing the right state on to the stack or not initializing the state correctly in the first place.