Adding Regex.IsMatch(Span) and RegexRunner.Scan() methods #65473

joperezr · 2022-02-17T00:15:36Z

This is mainly superseding #62509 now that the APIs have been approved. I have addressed most of the comments from @stephentoub there except for two things which I plan to do in this PR by adding one more commit which are:

Implement Regex.Run on top of Regex.Run in order to avoid duplication.
For most of the tests where we are testing isMatch with string, also call in the isMatch(span) new methods to get test coverage into the new surface area.

While I add the above two bullets into a commit, I thought of pushing this PR as it is ready for review other than those two things.

dotnet-issue-labeler · 2022-02-17T00:15:40Z

Note regarding the new-api-needs-documentation label:

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.

ghost · 2022-02-17T00:15:46Z

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #59629

This is mainly superseding #62509 now that the APIs have been approved. I have addressed most of the comments from @stephentoub there except for two things which I plan to do in this PR by adding one more commit which are:

Implement Regex.Run on top of Regex.Run in order to avoid duplication.
For most of the tests where we are testing isMatch with string, also call in the isMatch(span) new methods to get test coverage into the new surface area.

While I add the above two bullets into a commit, I thought of pushing this PR as it is ready for review other than those two things.

Author:	joperezr
Assignees:	-
Labels:	`area-System.Text.RegularExpressions`, `new-api-needs-documentation`
Milestone:	-

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs

src/libraries/System.Text.RegularExpressions/ref/System.Text.RegularExpressions.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

...raries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexInterpreter.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

stephentoub · 2022-02-17T03:03:55Z

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

+            // Everything below runs once per runner.
+
+            InitTrackCount();
+
+            int stacksize;
+            int tracksize = stacksize = runtrackcount * 8;
+
+            if (tracksize < 32)
+            {
+                tracksize = 32;
+            }
+            if (stacksize < 16)
+            {
+                stacksize = 16;
+            }
+
+            runtrack = new int[tracksize];
+            runtrackpos = tracksize;
+
+            runstack = new int[stacksize];
+            runstackpos = stacksize;
+
+            runcrawl = new int[32];
+            runcrawlpos = 32;


We don't need to do it now, but we're going to want to revisit this logic.

I assume what you are thinking is only do this initialization if the current runner is Interpreted right?

Partially. We only need to initialize runtrack if the runner is interpreted. We need runstack for Compiled and the source generator as well, but not for NonBacktracking, and the size is also based off of runtrackcount which is largely irrelevant, so we need to figure out what the right default size should be. runcrawl is only relevant if there are captures, so it's very likely the default size chosen here is too large.

… and adding a test for it.

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs

src/libraries/System.Text.RegularExpressions/src/Resources/Strings.resx

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

src/libraries/System.Text.RegularExpressions/tests/PrecompiledRegexScenarioTest.cs

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

src/libraries/System.Text.RegularExpressions/tests/PrecompiledRegexScenarioTest.cs

stephentoub

Getting close :)

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs

...tem.Text.RegularExpressions/src/System/Text/RegularExpressions/CompiledRegexRunnerFactory.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>

stephentoub

Some more mostly small comments to be addressed.

Other than that, LGTM. Thanks!

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Match.cs

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Capture.cs

…loads

joperezr · 2022-02-26T15:25:17Z

The remaining failure is unrelated to this PR, So I’ll go ahead and merge. I will also log an issue for that failure in case there isn’t one yet.

Adding Regex.IsMatch(Span) and RegexRunner.Scan() methods

27b0c72

joperezr requested review from stephentoub and danmoseley February 17, 2022 00:15

dotnet-issue-labeler bot added area-System.Text.RegularExpressions new-api-needs-documentation labels Feb 17, 2022

ghost assigned joperezr Feb 17, 2022

stephentoub reviewed Feb 17, 2022

View reviewed changes

joperezr added 5 commits February 17, 2022 14:17

Addressing some PR Feedback

6f933a3

Fixing case where span-based APIs are called from a precompiled regex…

7bfb28c

… and adding a test for it.

Adding Tests for IsMatch span overload

95c9f3d

Refactor Run<TState> and Run to share centralized logic

6a2043e

Emit IsBoundary and IsECMABoundary instead of exposing them.

c410b50

stephentoub reviewed Feb 18, 2022

View reviewed changes

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs Outdated Show resolved Hide resolved

stephentoub reviewed Feb 18, 2022

View reviewed changes

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs Outdated Show resolved Hide resolved

stephentoub reviewed Feb 18, 2022

View reviewed changes

src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs Outdated Show resolved Hide resolved

Address Emitter changes feedback.

6d1710b

stephentoub reviewed Feb 19, 2022

View reviewed changes

Addressing PR Feedback.

4ca5883