Fix missing shifting of Match._textxx fields (#72728)

This regressed during the work to support spans.  The Match._textxx fields were set based on the span positions rather than the input string positions, so if the input string had a non-zero beginning value, these fields would be off by that amount.  That then impacts NextMatch, as these values are fed into it.

While fixing this, I noticed that the _textstart field was wholely unnecessary and deleted it.  I also noticed we were unnecessarily passing around some values that weren't needed (e.g. a runtextbeg value that would always be 0), and re-storing the _regex object that can't ever change, and addressed those.

diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs
index ca41f06..5a4e65e 100644 (file)
--- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs
+++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Match.cs
@@ -38,11 +38,10 @@ namespace System.Text.RegularExpressions
         internal GroupCollection? _groupcoll;
 
         // input to the match
-        internal Regex? _regex;
-        internal int _textbeg;
-        internal int _textpos;
-        internal int _textend;
-        internal int _textstart;
+        internal readonly Regex? _regex;
+        internal int _textbeg; // 0 while the match is being generated, then bumped in Tidy to the actual beginning
+        internal int _textpos; // position offset from beginning
+        internal int _textend; // input length while the match is being generated, them bumped in Tidy to be the end
 
         // output from the match
         internal readonly int[][] _matches;
@@ -50,29 +49,25 @@ namespace System.Text.RegularExpressions
         internal bool _balancing;        // whether we've done any balancing with this match.  If we
                                          // have done balancing, we'll need to do extra work in Tidy().
 
-        internal Match(Regex? regex, int capcount, string? text, int begpos, int len, int startpos) :
+        internal Match(Regex? regex, int capcount, string? text, int textLength) :
             base(text, new int[2], 0, "0")
         {
             _regex = regex;
             _matchcount = new int[capcount];
             _matches = new int[capcount][];
             _matches[0] = _caps;
-            _textbeg = begpos;
-            _textend = begpos + len;
-            _textstart = startpos;
+            _textend = textLength;
             _balancing = false;
         }
 
         /// <summary>Returns an empty Match object.</summary>
-        public static Match Empty { get; } = new Match(null, 1, string.Empty, 0, 0, 0);
+        public static Match Empty { get; } = new Match(null, 1, string.Empty, 0);
 
-        internal void Reset(Regex regex, string? text, int textbeg, int textend, int textstart)
+        internal void Reset(string? text, int textLength)
         {
-            _regex = regex;
             Text = text;
-            _textbeg = textbeg;
-            _textend = textend;
-            _textstart = textstart;
+            _textbeg = 0;
+            _textend = textLength;
 
             int[] matchcount = _matchcount;
             for (int i = 0; i < matchcount.Length; i++)
@@ -278,7 +273,14 @@ namespace System.Text.RegularExpressions
 
             int[] matchcount = _matchcount;
             _capcount = matchcount[0]; // used to indicate Success
-            _textpos = textpos; // used to determine where to perform next match
+
+            // Used for NextMatch. During the matching process these stored the span-based offsets.
+            // If there was actually a beginning supplied, all of these need to be shifted by that
+            // beginning value.
+            Debug.Assert(_textbeg == 0);
+            _textbeg = beginningOfSpanSlice;
+            _textpos = beginningOfSpanSlice + textpos;
+            _textend += beginningOfSpanSlice;
 
             int[][] matches = _matches;
             int[] interval = matches[0];
@@ -380,8 +382,8 @@ namespace System.Text.RegularExpressions
     {
         private new readonly Hashtable _caps;
 
-        internal MatchSparse(Regex regex, Hashtable caps, int capcount, string? text, int begpos, int len, int startpos) :
-            base(regex, capcount, text, begpos, len, startpos)
+        internal MatchSparse(Regex regex, Hashtable caps, int capcount, string? text, int textLength) :
+            base(regex, capcount, text, textLength)
         {
             _caps = caps;
         }

diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs
index fdad5fc..3bed4d8 100644 (file)
--- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs
+++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexRunner.cs
@@ -243,16 +243,17 @@ namespace System.Text.RegularExpressions
             runtextend = text.Length;
             runtextpos = textstart;
 
-            if (runmatch is null)
+            Match? m = runmatch;
+            if (m is null)
             {
                 // Use a hashtabled Match object if the capture numbers are sparse
                 runmatch = runregex!.caps is null ?
-                    new Match(runregex, runregex.capsize, runtext, runtextbeg, runtextend - runtextbeg, runtextstart) :
-                    new MatchSparse(runregex, runregex.caps, runregex.capsize, runtext, runtextbeg, runtextend - runtextbeg, runtextstart);
+                    new Match(runregex, runregex.capsize, runtext, text.Length) :
+                    new MatchSparse(runregex, runregex.caps, runregex.capsize, runtext, text.Length);
             }
             else
             {
-                runmatch.Reset(runregex!, runtext, runtextbeg, runtextend, runtextstart);
+                m.Reset(runtext, text.Length);
             }
 
             // Note we test runcrawl, because it is the last one to be allocated

diff --git a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.MultipleMatches.Tests.cs b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.MultipleMatches.Tests.cs
index 65a3b5c..64f99d6 100644 (file)
--- a/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.MultipleMatches.Tests.cs
+++ b/src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.MultipleMatches.Tests.cs
@@ -506,5 +506,47 @@ namespace System.Text.RegularExpressions.Tests
         {
             Assert.Same(Match.Empty, Match.Empty.NextMatch());
         }
+
+        public static IEnumerable<object[]> Matches_NonZeroStartAtOrBeginning_MemberData()
+        {
+            foreach (RegexEngine engine in RegexHelpers.AvailableEngines)
+            {
+                yield return new object[] { engine, @"\.", @"a.b.c.d.e.f", 4, new (int, int)[] { (5, 1), (7, 1), (9, 1) } };
+                yield return new object[] { engine, @"\b\w+\b", @"ab cdef ghijk l mn opqr", 3, new (int, int)[] { (3, 4), (8, 5), (14, 1), (16, 2), (19, 4) } };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(Matches_NonZeroStartAtOrBeginning_MemberData))]
+        public async Task Matches_NonZeroStartAtOrBeginning(RegexEngine engine, string pattern, string input, int offset, (int Index, int Length)[] expected)
+        {
+            Regex r = await RegexHelpers.GetRegexAsync(engine, pattern);
+            int i;
+
+            MatchCollection matches = r.Matches(input, offset); // this overload takes startat, but none of the patterns used as input behave differently based on startat vs beginning
+            Assert.Equal(expected.Length, matches.Count);
+            for (i = 0; i < expected.Length; i++)
+            {
+                Assert.Equal(expected[i].Index, matches[i].Index);
+                Assert.Equal(expected[i].Length, matches[i].Length);
+            }
+
+            foreach (bool startat in new[] { true, false })
+            {
+                i = 0;
+                Match m = startat ?
+                    r.Match(input, offset) : // startat
+                    r.Match(input, offset, input.Length - offset); // beginning + length
+                while (m.Success)
+                {
+                    Assert.InRange(i, 0, expected.Length - 1);
+                    Assert.Equal(expected[i].Index, m.Index);
+                    Assert.Equal(expected[i].Length, m.Length);
+                    m = m.NextMatch();
+                    i++;
+                }
+                Assert.Equal(expected.Length, i);
+            }
+        }
     }
 }