Adding Count(ReadOnlySpan<char>) Overloads #66026
Conversation
ghost
added
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions Issue Detailsfixes #61425 This adds the remaining overloads for Count which work over the passed in span.
|
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Count.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs
Outdated
Show resolved
Hide resolved
|
The failures are unrelated (test failures of DLLImportGenerator which have been fixed in main). I'll rebase in order to be able to get a clean CI. |
| { | ||
| runner.runtext = null; // drop reference to text to avoid keeping it alive in a cache. | ||
| _runner = runner; | ||
| } |
There was a problem hiding this comment.
Not for this PR, but subsequently I think we should delete the try/finally. We never expect exceptions to come out of the execution, and if they do, the worst that happens is we lose the runner instance. Same with the other overloads.
| match.Text = null; | ||
| } | ||
| return; | ||
| if (usingStringOverload && reuseMatchObject) |
There was a problem hiding this comment.
Assigning null is pretty cheap. Do we functionally need this usingStringOverload, or could we just always null out match.Text if reuseMatchObject? If there's negligible perf difference, I'd rather keep it simpler. Might even be a tad faster by not having the extra argument and branch.
There was a problem hiding this comment.
Sorry, I missed this comment will send a short follow up addressing this one
2 participants
fixes #61425
This adds the remaining overloads for Count which work over the passed in span.
cc: @stephentoub @danmoseley