Skip to content

Commit

Permalink
Add a comment in source-generated regexes explaining why they cannot …
Browse files Browse the repository at this point in the history
…have optimized code. (dotnet#66114)

* Add a comment in source-generated regexes explaining why they cannot have optimized code.

* Apply suggestions from code review

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

Co-authored-by: Stephen Toub <stoub@microsoft.com>
  • Loading branch information
teo-tsirpanis and stephentoub committed Mar 3, 2022
1 parent b410984 commit 7db092a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ static uint ComputeStringHash(string s)
}

/// <summary>Gets whether a given regular expression method is supported by the code generator.</summary>
private static bool SupportsCodeGeneration(RegexMethod rm)
private static bool SupportsCodeGeneration(RegexMethod rm, out string? reason)
{
RegexNode root = rm.Tree.Root;

if (!root.SupportsCompilation())
if (!root.SupportsCompilation(out reason))
{
return false;
}
Expand All @@ -119,6 +119,7 @@ private static bool SupportsCodeGeneration(RegexMethod rm)
// 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.
reason = "the regex will result in code that may exceed C# compiler limits";
return false;
}

Expand Down Expand Up @@ -163,8 +164,10 @@ private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter wri
writer.Write(" public static global::System.Text.RegularExpressions.Regex Instance { get; } = ");

// If we can't support custom generation for this regex, spit out a Regex constructor call.
if (!SupportsCodeGeneration(rm))
if (!SupportsCodeGeneration(rm, out string? reason))
{
writer.WriteLine();
writer.WriteLine($"// Cannot generate Regex-derived implementation because {reason}.");
writer.WriteLine($"new global::System.Text.RegularExpressions.Regex({patternExpression}, {optionsExpression}, {timeoutExpression});");
writer.WriteLine("}");
return ImmutableArray.Create(Diagnostic.Create(DiagnosticDescriptors.LimitedSourceGeneration, rm.MethodSyntax.GetLocation()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal sealed class RegexLWCGCompiler : RegexCompiler
/// <summary>The top-level driver. Initializes everything then calls the Generate* methods.</summary>
public RegexRunnerFactory? FactoryInstanceFromCode(string pattern, RegexTree regexTree, RegexOptions options, bool hasTimeout)
{
if (!regexTree.Root.SupportsCompilation())
if (!regexTree.Root.SupportsCompilation(out _))
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ static RegexNode ExtractCommonPrefixText(RegexNode alternation)

// Now compare the rest of the branches against it.
int endingIndex = startingIndex + 1;
for ( ; endingIndex < children.Count; endingIndex++)
for (; endingIndex < children.Count; endingIndex++)
{
// Get the starting node of the next branch.
startingNode = children[endingIndex].FindBranchOneOrMultiStart();
Expand Down Expand Up @@ -2566,32 +2566,42 @@ public int ChildCount()
}

// Determines whether the node supports a compilation / code generation strategy based on walking the node tree.
internal bool SupportsCompilation()
// Also returns a human-readable string to explain the reason (it will be emitted by the source generator, hence
// there's no need to localize).
internal bool SupportsCompilation([NotNullWhen(false)] out string? reason)
{
if (!StackHelper.TryEnsureSufficientExecutionStack())
{
// If we can't recur further, code generation isn't supported as the tree is too deep.
reason = "run-time limits were exceeded";
return false;
}

if ((Options & (RegexOptions.RightToLeft | RegexOptions.NonBacktracking)) != 0)
// 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";
return false;
}

if ((Options & RegexOptions.RightToLeft) != 0)
{
// 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.
reason = "RegexOptions.RightToLeft or a positive/negative lookbehind was used";
return false;
}

int childCount = ChildCount();
for (int i = 0; i < childCount; i++)
{
// The node isn't supported if any of its children aren't supported.
if (!Child(i).SupportsCompilation())
if (!Child(i).SupportsCompilation(out reason))
{
return false;
}
}

// Supported.
reason = null;
return true;
}

Expand Down

0 comments on commit 7db092a

Please sign in to comment.