-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Remove unnecessary work from lazy loops (and a little from greedy) #68490
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
Conversation
- For a lazy loop, compute whether the child might match empty. Only if it might do we need any of the checks around empty iterations, and only in those situations do we need to track the starting position or whether we've seen an empty loop, so avoid all work associated with that if the min length of the child is greater than zero. - If the lazy loop failing will exit the match, avoid doing all the cleanup work that ends up being irrelevant. - If the lazy loop isn't itself part of another loop, we don't need the backtracking section to push/pop any additional state on the backtracking stack. - Avoid outputting two gotos in a row in greedy loops. - Avoid outputting additional if blocks in greedy loops when the checks are identical. - Add some comments, in particular to loops where the logic can be tricky and hard to follow / reason about. - Remove some emitted semicolons after labels. - Fix the naming of a few "SkipBacktrack" labels to conform to the same prefixed naming convention used for other labels
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue Details
Fixes #68415
|
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
Show resolved
Hide resolved
joperezr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I assume we already have more than enough coverage around all of these branches with our existing testbed.
I hope so :) We at least have these code paths covered. |
Fixes #68415