Refactor Regex FindFirstChar #60822
Refactor Regex FindFirstChar #60822
Conversation
|
Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions Issue DetailsIn the first commit, RegexCompiler/Emitter.EmitFindFirstChar are updated to separate out each of the strategies into their own function. There are no functional changes as part of this; the code is simply moved into a helper and then that helper invoked where the original code was. This will be easiest reviewed ignoring whitespace. In the second commit, RegexInterpreter.FindFirstChar is also updated. However, this does involve some functional changes. Rather than each call to FindFirstChar needing to figure out which strategy to use, we now compute that once in the RegexInterpreter ctor, and then in FindFirstChar just branch to the right place. I have some subsequent changes around vectorization for FindFirstChar I aim to make, and it was getting unwieldy doing so with the code as previously structured.
|
There was a problem hiding this comment.
(nit) why is the ! needed on code.LeadingCharClasses![0], when the line directly above already dereferences code.LeadingCharClasses?
There was a problem hiding this comment.
Leftover from when the code was written differently. Will remove.
...raries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexInterpreter.cs
Outdated
Show resolved
Hide resolved
These have become unwieldy, with multiple large code blocks for finding the first character at which to perform a full match, and we'll be adding more soon. This simply splits each of those out into their own method. No functional changes.
We don't need to re-evaluate which mechanism to use on every FindFirstChar call... we can do so once in the ctor and then just switch on that in the actual FindFirstChar call.
stephentoub
force-pushed
the
refactorfindfirst
branch
from
41e444d to
c668463
Compare
ghost
locked as resolved and limited conversation to collaborators
2 participants
In the first commit, RegexCompiler/Emitter.EmitFindFirstChar are updated to separate out each of the strategies into their own function. There are no functional changes as part of this; the code is simply moved into a helper and then that helper invoked where the original code was. This will be easiest reviewed ignoring whitespace.
In the second commit, RegexInterpreter.FindFirstChar is also updated. However, this does involve some functional changes. Rather than each call to FindFirstChar needing to figure out which strategy to use, we now compute that once in the RegexInterpreter ctor, and then in FindFirstChar just branch to the right place.
I have some subsequent changes around vectorization for FindFirstChar I aim to make, and it was getting unwieldy doing so with the code as previously structured.