Enable RegexOptions.RightToLeft and lookbehinds in compiler / source generator

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

@@ -143,7 +143,7 @@ private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, M
                 return Diagnostic.Create(DiagnosticDescriptors.InvalidLangVersion, methodSyntax.GetLocation());
             }
 
-            RegexOptions regexOptions = RegexOptions.Compiled | (options is not null ? (RegexOptions)options : RegexOptions.None);
+            RegexOptions regexOptions = options is not null ? (RegexOptions)options : RegexOptions.None;
 
             // TODO: This is going to include the culture that's current at the time of compilation.
             // What should we do about that?  We could:
@@ -181,7 +181,7 @@ private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, M
             RegexTree tree;
             try
             {
-                tree = RegexParser.Parse(pattern, regexOptions, culture);
+                tree = RegexParser.Parse(pattern, regexOptions | RegexOptions.Compiled, culture); // make sure Compiled is included to get all optimizations applied to it
             }
             catch (Exception e)
             {

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

@@ -50,7 +50,7 @@ public RegexFindOptimizations(RegexNode root, RegexOptions options, CultureInfo
 
             // Compute any anchor trailing the expression.  If there is one, and we can also compute a fixed length
             // for the whole expression, we can use that to quickly jump to the right location in the input.
-            if (!_rightToLeft) // haven't added FindNextStartingPositionMode support for RTL
+            if (!_rightToLeft) // haven't added FindNextStartingPositionMode trailing anchor support for RTL
             {
                 bool triedToComputeMaxLength = false;
 
@@ -297,16 +297,18 @@ public bool TryFindNextStartingPosition(ReadOnlySpan<char> textSpan, ref int pos
                 // For others, we can jump to the relevant location.
 
                 case FindNextStartingPositionMode.LeadingAnchor_LeftToRight_Beginning:
-                    if (pos > 0)
+                    if (pos != 0)
                     {
+                        // If we're not currently at the beginning, we'll never be, so fail immediately.
                         pos = textSpan.Length;
                         return false;
                     }
                     return true;
 
                 case FindNextStartingPositionMode.LeadingAnchor_LeftToRight_Start:
-                    if (pos > start)
+                    if (pos != start)
                     {
+                        // If we're not currently at the start, we'll never be, so fail immediately.
                         pos = textSpan.Length;
                         return false;
                     }
@@ -315,27 +317,38 @@ public bool TryFindNextStartingPosition(ReadOnlySpan<char> textSpan, ref int pos
                 case FindNextStartingPositionMode.LeadingAnchor_LeftToRight_EndZ:
                     if (pos < textSpan.Length - 1)
                     {
+                        // If we're not currently at the end (or a newline just before it), skip ahead
+                        // since nothing until then can possibly match.
                         pos = textSpan.Length - 1;
                     }
                     return true;
 
                 case FindNextStartingPositionMode.LeadingAnchor_LeftToRight_End:
                     if (pos < textSpan.Length)
                     {
+                        // If we're not currently at the end (or a newline just before it), skip ahead
+                        // since nothing until then can possibly match.
                         pos = textSpan.Length;
                     }
                     return true;
 
                 case FindNextStartingPositionMode.LeadingAnchor_RightToLeft_Beginning:
-                    if (pos > 0)
+                    if (pos != 0)
                     {
+                        // If we're not currently at the beginning, skip ahead (or, rather, backwards)
+                        // since nothing until then can possibly match. (We're iterating from the end
+                        // to the beginning in RightToLeft mode.)
                         pos = 0;
                     }
                     return true;
 
                 case FindNextStartingPositionMode.LeadingAnchor_RightToLeft_Start:
-                    if (pos < start)
+                    if (pos != start)
                     {
+                        // If we're not currently at the starting position, we'll never be, so fail immediately.
+                        // This is different from beginning, since beginning is the fixed location of 0 whereas
+                        // start is wherever the iteration actually starts from; in left-to-right, that's often
+                        // the same as beginning, but in RightToLeft it rarely is.
                         pos = 0;
                         return false;
                     }
@@ -344,6 +357,8 @@ public bool TryFindNextStartingPosition(ReadOnlySpan<char> textSpan, ref int pos
                 case FindNextStartingPositionMode.LeadingAnchor_RightToLeft_EndZ:
                     if (pos < textSpan.Length - 1 || ((uint)pos < (uint)textSpan.Length && textSpan[pos] != '\n'))
                     {
+                        // If we're not currently at the end, we'll never be (we're iterating from end to beginning),
+                        // so fail immediately.
                         pos = 0;
                         return false;
                     }
@@ -352,6 +367,8 @@ public bool TryFindNextStartingPosition(ReadOnlySpan<char> textSpan, ref int pos
                 case FindNextStartingPositionMode.LeadingAnchor_RightToLeft_End:
                     if (pos < textSpan.Length)
                     {
+                        // If we're not currently at the end, we'll never be (we're iterating from end to beginning),
+                        // so fail immediately.
                         pos = 0;
                         return false;
                     }

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

@@ -59,7 +59,7 @@ internal sealed class RegexLWCGCompiler : RegexCompiler
             EmitTryMatchAtCurrentPosition();
 
             DynamicMethod scanMethod = DefineDynamicMethod($"Regex{regexNum}_Scan{description}", null, typeof(CompiledRegexRunner), new[] { typeof(RegexRunner), typeof(ReadOnlySpan<char>) });
-            EmitScan(tryfindNextPossibleStartPositionMethod, tryMatchAtCurrentPositionMethod);
+            EmitScan(options, tryfindNextPossibleStartPositionMethod, tryMatchAtCurrentPositionMethod);
 
             return new CompiledRegexRunnerFactory(scanMethod);
         }

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

@@ -2558,39 +2558,46 @@ public int ChildCount()
         // there's no need to localize).
         internal bool SupportsCompilation([NotNullWhen(false)] out string? reason)
         {
-            if (!StackHelper.TryEnsureSufficientExecutionStack())
-            {
-                reason = "run-time limits were exceeded";
-                return false;
-            }
-
-            // NonBacktracking isn't supported, nor RightToLeft.  The latter applies to both the top-level
-            // options as well as when used to specify positive and negative lookbehinds.
             if ((Options & RegexOptions.NonBacktracking) != 0)
             {
-                reason = "RegexOptions.NonBacktracking was specified";
+                reason = "RegexOptions.NonBacktracking isn't supported";
                 return false;
             }
 
-            if ((Options & RegexOptions.RightToLeft) != 0)
+            if (ExceedsMaxDepthAllowedDepth(this, allowedDepth: 40))
             {
-                reason = "RegexOptions.RightToLeft or a positive/negative lookbehind was used";
+                // For the source generator, deep RegexNode trees can result in emitting C# code that exceeds C# compiler
+                // limitations, leading to "CS8078: An expression is too long or complex to compile". As such, we place
+                // an artificial limit on max tree depth in order to mitigate such issues. The allowed depth can be tweaked
+                // as needed; its exceedingly rare to find expressions with such deep trees. And while RegexCompiler doesn't
+                // have to deal with C# compiler limitations, we still want to limit max tree depth as we want to limit
+                // how deep recursion we'll employ as part of code generation.
+                reason = "the expression may result exceeding run-time or compiler limits";
                 return false;
             }
 
-            int childCount = ChildCount();
-            for (int i = 0; i < childCount; i++)
+            // Supported.
+            reason = null;
+            return true;
+
+            static bool ExceedsMaxDepthAllowedDepth(RegexNode node, int allowedDepth)
             {
-                // The node isn't supported if any of its children aren't supported.
-                if (!Child(i).SupportsCompilation(out reason))
+                if (allowedDepth <= 0)
                 {
-                    return false;
+                    return true;
                 }
-            }
 
-            // Supported.
-            reason = null;
-            return true;
+                int childCount = node.ChildCount();
+                for (int i = 0; i < childCount; i++)
+                {
+                    if (ExceedsMaxDepthAllowedDepth(node.Child(i), allowedDepth - 1))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
         }
 
         /// <summary>Gets whether the node is a Set/Setloop/Setloopatomic/Setlazy node.</summary>

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

@@ -23,8 +23,9 @@ static bool TryAnalyze(RegexNode node, AnalysisResults results, bool isAtomicByA
                     return false;
                 }
 
-                // Track whether we've seen any node with IgnoreCase set.
+                // Track whether we've seen any nodes with various options set.
                 results._hasIgnoreCase |= (node.Options & RegexOptions.IgnoreCase) != 0;
+                results._hasRightToLeft |= (node.Options & RegexOptions.RightToLeft) != 0;
 
                 if (isAtomicByAncestor)
                 {
@@ -149,6 +150,8 @@ internal sealed class AnalysisResults
         internal HashSet<RegexNode>? _mayBacktrack;
         /// <summary>Whether any node has <see cref="RegexOptions.IgnoreCase"/> set.</summary>
         internal bool _hasIgnoreCase;
+        /// <summary>Whether any node has <see cref="RegexOptions.RightToLeft"/> set.</summary>
+        internal bool _hasRightToLeft;
 
         /// <summary>Initializes the instance.</summary>
         /// <param name="regexTree">The code being analyzed.</param>
@@ -158,23 +161,48 @@ internal sealed class AnalysisResults
         public RegexTree RegexTree { get; }
 
         /// <summary>Gets whether a node is considered atomic based on its ancestry.</summary>
+        /// <remarks>
+        /// If the whole tree couldn't be examined, this returns false.  That could lead to additional
+        /// code being output as nodes that could have been made atomic aren't, but functionally it's
+        /// the safe choice.
+        /// </remarks>
         public bool IsAtomicByAncestor(RegexNode node) => _isAtomicByAncestor.Contains(node);
 
         /// <summary>Gets whether a node directly or indirectly contains captures.</summary>
+        /// <remarks>
+        /// If the whole tree couldn't be examined, this returns true.  That could lead to additional
+        /// code being emitted to deal with captures that can't occur, but functionally it's the
+        /// safe choice.
+        /// </remarks>
         public bool MayContainCapture(RegexNode node) => !_complete || _containsCapture.Contains(node);
 
         /// <summary>Gets whether a node is or directory or indirectly contains a backtracking construct that isn't hidden by an internal atomic construct.</summary>
         /// <remarks>
         /// In most code generation situations, we only need to know after we emit the child code whether
         /// the child may backtrack, and that we can see with 100% certainty.  This method is useful in situations
         /// where we need to predict without traversing the child at code generation time whether it may
-        /// incur backtracking.  This method may have (few) false positives, but won't have any false negatives,
+        /// incur backtracking.  This method may have (few) false positives (return true when it could have
+        /// returned false), but won't have any false negatives (return false when it should have returned true),
         /// meaning it might claim a node requires backtracking even if it doesn't, but it will always return
-        /// true for any node that requires backtracking.
+        /// true for any node that requires backtracking. In that vein, if the whole tree couldn't be examined,
+        /// this returns true.
         /// </remarks>
         public bool MayBacktrack(RegexNode node) => !_complete || (_mayBacktrack?.Contains(node) ?? false);
 
         /// <summary>Gets whether a node might have <see cref="RegexOptions.IgnoreCase"/> set.</summary>
-        public bool HasIgnoreCase => _complete && _hasIgnoreCase;
+        /// <remarks>
+        /// If the whole tree couldn't be examined, this returns true.  That could lead to additional
+        /// code being emitted to support case-insensitivity in expressions that don't actually need
+        /// it, but functionally it's the safe choice.
+        /// </remarks>
+        public bool HasIgnoreCase => !_complete || _hasIgnoreCase;
+
+        /// <summary>Gets whether a node might have <see cref="RegexOptions.RightToLeft"/> set.</summary>
+        /// <remarks>
+        /// If the whole tree couldn't be examined, this returns true.  That could lead to additional
+        /// code being emitted to support expressions that don't actually contain any RightToLeft
+        /// nodes, but functionally it's the safe choice.
+        /// </remarks>
+        public bool HasRightToLeft => !_complete || _hasRightToLeft;
     }
 }

src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

@@ -84,11 +84,36 @@ public static IEnumerable<object[]> Match_MemberData()
 
                 if (!RegexHelpers.IsNonBacktracking(engine))
                 {
-                    // Zero-width negative lookahead assertion: Actual - "abc(?!XXX)\\w+"
+                    // Zero-width negative lookahead assertion
                     yield return (@"abc(?!XXX)\w+", "abcXXXdef", RegexOptions.None, 0, 9, false, string.Empty);
 
-                    // Zero-width positive lookbehind assertion: Actual - "(\\w){6}(?<=XXX)def"
+                    // Zero-width positive lookbehind assertion
                     yield return (@"(\w){6}(?<=XXX)def", "abcXXXdef", RegexOptions.None, 0, 9, true, "abcXXXdef");
+                    yield return (@"(?<=c)def", "123abcdef", RegexOptions.None, 0, 9, true, "def");
+                    yield return (@"(?<=abc)def", "123abcdef", RegexOptions.None, 0, 9, true, "def");
+                    yield return (@"(?<=a\wc)def", "123abcdef", RegexOptions.None, 0, 9, true, "def");
+                    yield return (@"(?<=\ba\wc)def", "123 abcdef", RegexOptions.None, 0, 10, true, "def");
+                    yield return (@"(?<=.\ba\wc\B)def", "123 abcdef", RegexOptions.None, 0, 10, true, "def");
+                    yield return (@"(?<=^123 abc)def", "123 abcdef", RegexOptions.None, 0, 10, true, "def");
+                    yield return (@"(?<=^123 abc)def", "123 abcdef", RegexOptions.Multiline, 0, 10, true, "def");
+                    yield return (@"(?<=123$\nabc)def", "123\nabcdef", RegexOptions.Multiline, 0, 10, true, "def");
+                    yield return (@"123(?<!$) abcdef", "123 abcdef", RegexOptions.None, 0, 10, true, "123 abcdef");
+                    yield return (@"\w{3}(?<=^xyz|^abc)defg", "abcdefg", RegexOptions.None, 0, 7, true, "abcdefg");
+                    yield return (@"(abc)\w(?<=(?(1)d|e))", "abcdabc", RegexOptions.None, 0, 7, true, "abcd");
+                    yield return (@"(abc)\w(?<!(?(1)e|d))", "abcdabc", RegexOptions.None, 0, 7, true, "abcd");
+                    yield return (@"(abc)\w(?<=(?(cd)d|e))", "abcdabc", RegexOptions.None, 0, 7, true, "abcd");
+                    yield return (@"(abc)\w(?<!(?(cd)e|d))", "abcdabc", RegexOptions.None, 0, 7, true, "abcd");
+                    yield return (@"\w{3}(?<=(\w){6,8})", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "def");
+                    yield return (@"\w{3}(?<=(?:\d\w){4})", "a1b2c3d4e5", RegexOptions.None, 0, 10, true, "d4e");
+                    yield return (@"\w{3}(?<=(\w){6,8}?)", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "def");
+                    yield return (@"\w{3}(?<=(?:\d\w){3,4}?\d)", "a1b2c3d4e5", RegexOptions.None, 0, 10, true, "3d4");
+                    yield return (@"(\w{3})\w*(?<=\1)", "---abcdefababc123", RegexOptions.None, 0, 17, true, "abcdefababc");
+                    yield return (@"(?<=\w{3})\w{4}", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "defg");
+                    yield return (@"(?<=\w{3,8})\w{4}", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "defg");
+                    yield return (@"(?<=\w*)\w{4}", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "abcd");
+                    yield return (@"(?<=\w?)\w{4}", "abcdefghijklmnop", RegexOptions.None, 0, 16, true, "abcd");
+                    yield return (@"(?<=\d?)a{4}", "123aaaaaaaaa", RegexOptions.None, 0, 12, true, "aaaa");
+                    yield return (@"(?<=a{3,5}[ab]*)1234", "aaaaaaa1234", RegexOptions.None, 0, 11, true, "1234");
 
                     // Zero-width negative lookbehind assertion: Actual - "(\\w){6}(?<!XXX)def"
                     yield return (@"(\w){6}(?<!XXX)def", "XXXabcdef", RegexOptions.None, 0, 9, true, "XXXabcdef");

src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/RegexGeneratorParserTests.cs

@@ -197,21 +197,6 @@ partial class C
             Assert.Equal("SYSLIB1044", Assert.Single(diagnostics).Id);
         }
 
-        [Fact]
-        public async Task Diagnostic_RightToLeft_LimitedSupport()
-        {
-            IReadOnlyList<Diagnostic> diagnostics = await RegexGeneratorHelper.RunGenerator(@"
-                using System.Text.RegularExpressions;
-                partial class C
-                {
-                    [RegexGenerator(""ab"", RegexOptions.RightToLeft)]
-                    private static partial Regex RightToLeftNotSupported();
-                }
-            ");
-
-            Assert.Equal("SYSLIB1045", Assert.Single(diagnostics).Id);
-        }
-
         [Fact]
         public async Task Diagnostic_NonBacktracking_LimitedSupport()
         {
@@ -227,36 +212,6 @@ partial class C
             Assert.Equal("SYSLIB1045", Assert.Single(diagnostics).Id);
         }
 
-        [Fact]
-        public async Task Diagnostic_PositiveLookbehind_LimitedSupport()
-        {
-            IReadOnlyList<Diagnostic> diagnostics = await RegexGeneratorHelper.RunGenerator(@"
-                using System.Text.RegularExpressions;
-                partial class C
-                {
-                    [RegexGenerator(""(?<=\b20)\d{2}\b"")]
-                    private static partial Regex PositiveLookbehindNotSupported();
-                }
-            ");
-
-            Assert.Equal("SYSLIB1045", Assert.Single(diagnostics).Id);
-        }
-
-        [Fact]
-        public async Task Diagnostic_NegativeLookbehind_LimitedSupport()
-        {
-            IReadOnlyList<Diagnostic> diagnostics = await RegexGeneratorHelper.RunGenerator(@"
-                using System.Text.RegularExpressions;
-                partial class C
-                {
-                    [RegexGenerator(""(?<!(Saturday|Sunday) )\b\w+ \d{1,2}, \d{4}\b"")]
-                    private static partial Regex NegativeLookbehindNotSupported();
-                }
-            ");
-
-            Assert.Equal("SYSLIB1045", Assert.Single(diagnostics).Id);
-        }
-
         [Fact]
         public async Task Valid_ClassWithoutNamespace()
         {