Support auto-atomicity on {lazy} loops

Today we only support conversion of loops to atomic loops for single-character loops (e.g. a* or [abc]*). This PR augments the logic to support arbitrary loops, enabling many more loops to become atomic.

Author: stephentoub

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

@@ -154,7 +154,7 @@ private void MakeRep(RegexNodeKind kind, int min, int max)
             N = max;
         }
 
-        private void MakeLoopAtomic()
+        private void MakeLoopAtomic(bool isEnding = false)
         {
             switch (Kind)
             {
@@ -165,11 +165,15 @@ private void MakeLoopAtomic()
                     break;
 
                 case RegexNodeKind.Onelazy or RegexNodeKind.Notonelazy or RegexNodeKind.Setlazy:
-                    // For lazy, we not only change the Type, we also lower the max number of iterations
-                    // to the minimum number of iterations, creating a repeater, as they should end up
-                    // matching as little as possible.
+                    // For lazy, we not only change the Type, if we're at the end of the pattern,
+                    // we also lower the max number of iterations to the minimum number of iterations,
+                    // creating a repeater, as they should end up matching as little as possible.
                     Kind += RegexNodeKind.Oneloopatomic - RegexNodeKind.Onelazy;
-                    N = M;
+                    if (isEnding)
+                    {
+                        N = M;
+                    }
+
                     if (N == 0)
                     {
                         // If moving the max to be the same as the min dropped it to 0, there's no
@@ -189,6 +193,40 @@ private void MakeLoopAtomic()
                     }
                     break;
 
+                case RegexNodeKind.Lazyloop:
+                    // For lazy, we not only change the Type, if we're at the end of the pattern,
+                    // we also lower the max number of iterations to the minimum number of iterations,
+                    // creating a repeater, as they should end up matching as little as possible.
+                    if (isEnding)
+                    {
+                        N = M;
+                    }
+
+                    if (N == 0)
+                    {
+                        // If moving the max to be the same as the min dropped it to 0, there's no
+                        // work to be done for this node, and we can make it Empty.
+                        Kind = RegexNodeKind.Empty;
+                        Children = null;
+                    }
+                    else
+                    {
+                        // change it to be a greedy loop
+                        goto case RegexNodeKind.Loop;
+                    }
+                    break;
+
+                case RegexNodeKind.Loop:
+                    if (Parent is not { Kind: RegexNodeKind.Atomic })
+                    {
+                        RegexNode loopAsChild = new(RegexNodeKind.Loop, Options, M, N);
+                        Kind = RegexNodeKind.Atomic;
+                        M = N = 0;
+                        loopAsChild.AddChild(Child(0));
+                        ReplaceChild(0, loopAsChild);
+                    }
+                    break;
+
                 default:
                     Debug.Fail($"Unexpected type: {Kind}");
                     break;
@@ -430,7 +468,7 @@ private void EliminateEndingBacktracking()
                     // or even empty nodes.
                     case RegexNodeKind.Oneloop or RegexNodeKind.Notoneloop or RegexNodeKind.Setloop:
                     case RegexNodeKind.Onelazy or RegexNodeKind.Notonelazy or RegexNodeKind.Setlazy:
-                        node.MakeLoopAtomic();
+                        node.MakeLoopAtomic(isEnding: true);
                         break;
 
                     // Just because a particular node is atomic doesn't mean all its descendants are.
@@ -451,7 +489,7 @@ private void EliminateEndingBacktracking()
                     case RegexNodeKind.Concatenate:
                         RegexNode existingChild = node.Child(node.ChildCount() - 1);
                         if ((existingChild.Kind is RegexNodeKind.Alternate or RegexNodeKind.BackreferenceConditional or RegexNodeKind.ExpressionConditional or RegexNodeKind.Loop or RegexNodeKind.Lazyloop) &&
-                            (node.Parent is null || node.Parent.Kind != RegexNodeKind.Atomic)) // validate grandparent isn't atomic
+                            node.Parent is not { Kind: RegexNodeKind.Atomic }) // validate grandparent isn't atomic
                         {
                             var atomic = new RegexNode(RegexNodeKind.Atomic, existingChild.Options);
                             atomic.AddChild(existingChild);
@@ -493,20 +531,33 @@ private void EliminateEndingBacktracking()
                         goto case RegexNodeKind.Loop;
                     case RegexNodeKind.Loop:
                         {
-                            if (node.N == 1)
+                            // Make the loop atomic, if it isn't already. This entails changing node to instead be an Atomic node
+                            // that has the {Lazy}Loop as its child. If the parent of the loop is already Atomic, this will be a nop.
+                            node.MakeLoopAtomic(isEnding: true);
+                            Debug.Assert(node.Kind is RegexNodeKind.Atomic or RegexNodeKind.Empty or RegexNodeKind.Loop or RegexNodeKind.Lazyloop);
+
+                            if (node.Kind is RegexNodeKind.Atomic)
                             {
-                                // If the loop has a max iteration count of 1 (e.g. it's an optional node),
-                                // there's no possibility for conflict between multiple iterations, so
-                                // we can process it.
                                 node = node.Child(0);
-                                continue;
                             }
 
-                            RegexNode? loopDescendent = node.FindLastExpressionInLoopForAutoAtomic();
-                            if (loopDescendent != null)
+                            if (node.Kind is RegexNodeKind.Loop or RegexNodeKind.Lazyloop)
                             {
-                                node = loopDescendent;
-                                continue; // loop around to process node
+                                if (node.N == 1)
+                                {
+                                    // If the loop has a max iteration count of 1 (e.g. it's an optional node),
+                                    // there's no possibility for conflict between multiple iterations, so
+                                    // we can process it.
+                                    node = node.Child(0);
+                                    continue;
+                                }
+
+                                RegexNode? loopDescendent = node.FindLastExpressionInLoopForAutoAtomic();
+                                if (loopDescendent != null)
+                                {
+                                    node = loopDescendent;
+                                    continue; // loop around to process node
+                                }
                             }
                         }
                         break;
@@ -635,7 +686,7 @@ private RegexNode ReduceAtomic()
                 case RegexNodeKind.Onelazy:
                 case RegexNodeKind.Notonelazy:
                 case RegexNodeKind.Setlazy:
-                    child.MakeLoopAtomic();
+                    child.MakeLoopAtomic(isEnding: true);
                     return child;
 
                 // Alternations have a variety of possible optimizations that can be applied
@@ -1876,7 +1927,7 @@ static void ProcessNode(RegexNode node, RegexNode subsequent)
                     // If the node can be changed to atomic based on what comes after it, do so.
                     switch (node.Kind)
                     {
-                        case RegexNodeKind.Oneloop or RegexNodeKind.Notoneloop or RegexNodeKind.Setloop when CanBeMadeAtomic(node, subsequent, iterateNullableSubsequent: true, allowLazy: false):
+                        case RegexNodeKind.Oneloop or RegexNodeKind.Notoneloop or RegexNodeKind.Setloop or RegexNodeKind.Loop when CanBeMadeAtomic(node, subsequent, iterateNullableSubsequent: true, allowLazy: false):
                             // The greedy loop doesn't overlap with what comes after it, which means giving anything it matches back will not
                             // help the overall match to succeed, which means it can simply become atomic to match as much as possible. The call
                             // to CanBeMadeAtomic passes iterateNullableSubsequent=true because, in a pattern like a*b*c*, when analyzing a*, we
@@ -1885,7 +1936,7 @@ static void ProcessNode(RegexNode node, RegexNode subsequent)
                             node.MakeLoopAtomic();
                             break;
 
-                        case RegexNodeKind.Onelazy or RegexNodeKind.Notonelazy or RegexNodeKind.Setlazy when CanBeMadeAtomic(node, subsequent, iterateNullableSubsequent: false, allowLazy: true):
+                        case RegexNodeKind.Onelazy or RegexNodeKind.Notonelazy or RegexNodeKind.Setlazy or RegexNodeKind.Lazyloop when CanBeMadeAtomic(node, subsequent, iterateNullableSubsequent: false, allowLazy: true):
                             // The lazy loop doesn't overlap with what comes after it, which means it needs to match as much as its allowed
                             // to match in order for there to be a possibility that what comes next matches (if it doesn't match as much
                             // as it's allowed and there was still more it could match, then what comes next is guaranteed to not match,
@@ -1901,7 +1952,10 @@ static void ProcessNode(RegexNode node, RegexNode subsequent)
                             // allowLazy is set to true so that the implementation will analyze rather than ignore this node; generally lazy nodes
                             // are ignored due to making them atomic not generally being a sound change, but here we're explicitly choosing to
                             // given the circumstances.
-                            node.Kind -= RegexNodeKind.Onelazy - RegexNodeKind.Oneloop; // lazy to greedy
+                            if (node.Kind is not RegexNodeKind.Lazyloop)
+                            {
+                                node.Kind -= RegexNodeKind.Onelazy - RegexNodeKind.Oneloop; // lazy to greedy
+                            }
                             node.MakeLoopAtomic();
                             break;
 
@@ -2193,6 +2247,52 @@ private static bool CanBeMadeAtomic(RegexNode node, RegexNode subsequent, bool i
                         }
                         break;
 
+                    case RegexNodeKind.Loop:
+                    case RegexNodeKind.Lazyloop when allowLazy:
+                        // With single character loops (e.g. OneLoop, NotOneLoop, SetLoop), we only need to prove there's no overlap between
+                        // what that single character could be and what comes next. For arbitrary loops, we have more to prove. First, we need
+                        // to understand what the loop can possibly start with and what it can possibly end with (with a single character loop,
+                        // those are the same things), and we need to ensure that there's no overlap between those two sets; otherwise, a second
+                        // iteration of a loop could end up giving back characters that could be consumed by the previous iteration. Second, we need
+                        // to ensure that neither the starting set nor the ending set overlaps with what could possibly come after it, for the same reason.
+                        if (RegexPrefixAnalyzer.FindFirstCharClass(node) is not string loopStartingSet ||
+                            RegexPrefixAnalyzer.FindLastCharClass(node) is not string loopEndingSet ||
+                            RegexCharClass.MayOverlap(loopStartingSet, loopEndingSet))
+                        {
+                            return false;
+                        }
+
+                        bool CharInStartingOrEndingSet(char ch) =>
+                            RegexCharClass.CharInClass(ch, loopStartingSet) || RegexCharClass.CharInClass(ch, loopEndingSet);
+
+                        bool MayOverlapStartingOrEndingSet(string set) =>
+                            RegexCharClass.MayOverlap(set, loopStartingSet) || RegexCharClass.MayOverlap(set, loopEndingSet);
+
+                        switch (subsequent.Kind)
+                        {
+                            case RegexNodeKind.One when !CharInStartingOrEndingSet(subsequent.Ch):
+                            case RegexNodeKind.Set when !MayOverlapStartingOrEndingSet(subsequent.Str!):
+                            case RegexNodeKind.Onelazy or RegexNodeKind.Oneloop or RegexNodeKind.Oneloopatomic when subsequent.M > 0 && !CharInStartingOrEndingSet(subsequent.Ch):
+                            case RegexNodeKind.Setlazy or RegexNodeKind.Setloop or RegexNodeKind.Setloopatomic when subsequent.M > 0 && !MayOverlapStartingOrEndingSet(subsequent.Str!):
+                            case RegexNodeKind.Multi when !CharInStartingOrEndingSet(subsequent.Str![0]):
+                            case RegexNodeKind.End:
+                            case RegexNodeKind.EndZ or RegexNodeKind.Eol when !CharInStartingOrEndingSet('\n'):
+                                return true;
+
+                            case RegexNodeKind.Onelazy or RegexNodeKind.Oneloop or RegexNodeKind.Oneloopatomic when subsequent.M == 0 && !CharInStartingOrEndingSet(subsequent.Ch):
+                            case RegexNodeKind.Setlazy or RegexNodeKind.Setloop or RegexNodeKind.Setloopatomic when subsequent.M == 0 && !MayOverlapStartingOrEndingSet(subsequent.Str!):
+                            case RegexNodeKind.Boundary when node.M > 0 && RegexCharClass.IsKnownWordClassSubset(loopStartingSet) && RegexCharClass.IsKnownWordClassSubset(loopEndingSet):
+                            case RegexNodeKind.NonBoundary when node.M > 0 && (loopStartingSet is RegexCharClass.NotWordClass or RegexCharClass.NotDigitClass) && (loopEndingSet is RegexCharClass.NotWordClass or RegexCharClass.NotDigitClass):
+                            case RegexNodeKind.ECMABoundary when node.M > 0 && (loopStartingSet is RegexCharClass.ECMAWordClass or RegexCharClass.ECMADigitClass) && (loopEndingSet is RegexCharClass.ECMAWordClass or RegexCharClass.ECMADigitClass):
+                            case RegexNodeKind.NonECMABoundary when node.M > 0 && (loopStartingSet is RegexCharClass.NotECMAWordClass or RegexCharClass.NotDigitClass) && (loopEndingSet is RegexCharClass.NotECMAWordClass or RegexCharClass.NotDigitClass):
+                                // The loop can be made atomic based on this subsequent node, but we'll need to evaluate the next one as well.
+                                break;
+
+                            default:
+                                return false;
+                        }
+                        break;
+
                     default:
                         return false;
                 }