Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize the Regex source generator's handling of Compilation objects. #65431

Merged
merged 4 commits into from
Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public partial class RegexGenerator
};

/// <summary>Generates the code for one regular expression class.</summary>
private static (string, ImmutableArray<Diagnostic>) EmitRegexType(RegexType regexClass, Compilation compilation)
private static (string, ImmutableArray<Diagnostic>) EmitRegexType(RegexType regexClass, bool allowUnsafe)
{
var sb = new StringBuilder(1024);
var writer = new IndentedTextWriter(new StringWriter(sb));
Expand Down Expand Up @@ -78,7 +78,7 @@ private static (string, ImmutableArray<Diagnostic>) EmitRegexType(RegexType rege
generatedName += ComputeStringHash(generatedName).ToString("X");

// Generate the regex type
ImmutableArray<Diagnostic> diagnostics = EmitRegexMethod(writer, regexClass.Method, generatedName, compilation);
ImmutableArray<Diagnostic> diagnostics = EmitRegexMethod(writer, regexClass.Method, generatedName, allowUnsafe);

while (writer.Indent != 0)
{
Expand Down Expand Up @@ -145,7 +145,7 @@ static bool ExceedsMaxDepthForSimpleCodeGeneration(RegexNode node, int allowedDe
}

/// <summary>Generates the code for a regular expression method.</summary>
private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter writer, RegexMethod rm, string id, Compilation compilation)
private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter writer, RegexMethod rm, string id, bool allowUnsafe)
{
string patternExpression = Literal(rm.Pattern);
string optionsExpression = Literal(rm.Options);
Expand All @@ -170,8 +170,6 @@ private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter wri
return ImmutableArray.Create(Diagnostic.Create(DiagnosticDescriptors.LimitedSourceGeneration, rm.MethodSyntax.GetLocation()));
}

bool allowUnsafe = compilation.Options is CSharpCompilationOptions { AllowUnsafe: true };

writer.WriteLine($"new {id}();");
writer.WriteLine();
writer.WriteLine($" private {id}()");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.DotnetRuntime.Extensions;
using System;
using System.Collections;
using System.Collections.Generic;
Expand All @@ -21,37 +20,35 @@ public partial class RegexGenerator
private const string RegexName = "System.Text.RegularExpressions.Regex";
private const string RegexGeneratorAttributeName = "System.Text.RegularExpressions.RegexGeneratorAttribute";

private static bool IsSyntaxTargetForGeneration(SyntaxNode node) =>
private static bool IsSyntaxTargetForGeneration(SyntaxNode node, CancellationToken cancellationToken) =>
// We don't have a semantic model here, so the best we can do is say whether there are any attributes.
node is MethodDeclarationSyntax { AttributeLists: { Count: > 0 } };

private static MethodDeclarationSyntax? GetSemanticTargetForGeneration(GeneratorSyntaxContext context)
private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, MethodDeclarationSyntax methodDeclarationSyntax, CancellationToken cancellationToken)
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
{
var methodDeclarationSyntax = (MethodDeclarationSyntax)context.Node;

foreach (AttributeListSyntax attributeListSyntax in methodDeclarationSyntax.AttributeLists)
{
foreach (AttributeSyntax attributeSyntax in attributeListSyntax.Attributes)
{
if (context.SemanticModel.GetSymbolInfo(attributeSyntax).Symbol is IMethodSymbol attributeSymbol &&
if (semanticModel.GetSymbolInfo(attributeSyntax, cancellationToken).Symbol is IMethodSymbol attributeSymbol &&
attributeSymbol.ContainingType.ToDisplayString() == RegexGeneratorAttributeName)
{
return methodDeclarationSyntax;
return true;
}
}
}

return null;
return false;
}

// Returns null if nothing to do, Diagnostic if there's an error to report, or RegexType if the type was analyzed successfully.
private static object? GetRegexTypeToEmit(Compilation compilation, MethodDeclarationSyntax methodSyntax, CancellationToken cancellationToken)
private static object? GetSemanticTargetForGeneration(GeneratorSyntaxContext context, CancellationToken cancellationToken)
{
INamedTypeSymbol? regexSymbol = compilation.GetBestTypeByMetadataName(RegexName);
INamedTypeSymbol? regexGeneratorAttributeSymbol = compilation.GetBestTypeByMetadataName(RegexGeneratorAttributeName);
if (regexSymbol is null || regexGeneratorAttributeSymbol is null)
SemanticModel sm = context.SemanticModel;
var methodSyntax = (MethodDeclarationSyntax)context.Node;

if (!IsSemanticTargetForGeneration(sm, methodSyntax, cancellationToken))
{
// Required types aren't available
return null;
}

Expand All @@ -61,8 +58,6 @@ private static bool IsSyntaxTargetForGeneration(SyntaxNode node) =>
return null;
}

SemanticModel sm = compilation.GetSemanticModel(methodSyntax.SyntaxTree);

IMethodSymbol? regexMethodSymbol = sm.GetDeclaredSymbol(methodSyntax, cancellationToken) as IMethodSymbol;
if (regexMethodSymbol is null)
{
Expand All @@ -81,7 +76,7 @@ private static bool IsSyntaxTargetForGeneration(SyntaxNode node) =>
int? matchTimeout = null;
foreach (AttributeData attributeData in boundAttributes)
{
if (attributeData.AttributeClass?.Equals(regexGeneratorAttributeSymbol) != true)
if (attributeData.AttributeClass?.ToDisplayString()?.Equals(RegexGeneratorAttributeName, StringComparison.Ordinal) != true)
teo-tsirpanis marked this conversation as resolved.
Show resolved Hide resolved
{
continue;
}
Expand Down Expand Up @@ -127,7 +122,7 @@ private static bool IsSyntaxTargetForGeneration(SyntaxNode node) =>
if (!regexMethodSymbol.IsPartialDefinition ||
regexMethodSymbol.Parameters.Length != 0 ||
regexMethodSymbol.Arity != 0 ||
!regexMethodSymbol.ReturnType.Equals(regexSymbol))
!regexMethodSymbol.ReturnType.ToDisplayString().Equals(RegexName, StringComparison.Ordinal))
{
return Diagnostic.Create(DiagnosticDescriptors.RegexMethodMustHaveValidSignature, methodSyntax.GetLocation());
}
Expand Down
45 changes: 14 additions & 31 deletions src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,30 @@ public partial class RegexGenerator : IIncrementalGenerator
{
public void Initialize(IncrementalGeneratorInitializationContext context)
{
// To avoid invalidating the generator's output when anything from the compilation
// changes, we will extract from it the only thing we care about: whether unsafe
// code is allowed.
IncrementalValueProvider<bool> allowUnsafeProvider =
context.CompilationProvider
.Select((x, _) => x.Options is CSharpCompilationOptions { AllowUnsafe: true });

// Contains one entry per regex method, either the generated code for that regex method,
// a diagnostic to fail with, or null if no action should be taken for that regex.
IncrementalValueProvider<ImmutableArray<object?>> codeOrDiagnostics =
context.SyntaxProvider

// Find all MethodDeclarationSyntax nodes attributed with RegexGenerator
.CreateSyntaxProvider(static (s, _) => IsSyntaxTargetForGeneration(s), static (ctx, _) => GetSemanticTargetForGeneration(ctx))
// Find all MethodDeclarationSyntax nodes attributed with RegexGenerator and gather the required information
.CreateSyntaxProvider(IsSyntaxTargetForGeneration, GetSemanticTargetForGeneration)
.Where(static m => m is not null)

// Pair each with the compilation
.Combine(context.CompilationProvider)

// Use a custom comparer that ignores the compilation. We want to avoid regenerating for regex methods
// that haven't been changed, but any change to a regex method will change the Compilation, so we ignore
// the Compilation for purposes of caching.
.WithComparer(new LambdaComparer<(MethodDeclarationSyntax?, Compilation)>(
static (left, right) => EqualityComparer<MethodDeclarationSyntax>.Default.Equals(left.Item1, right.Item1),
static o => o.Item1?.GetHashCode() ?? 0))
// Pair each with whether unsafe code is allowed
.Combine(allowUnsafeProvider)

// Get the resulting code string or error Diagnostic for each MethodDeclarationSyntax/Compilation pair
teo-tsirpanis marked this conversation as resolved.
Show resolved Hide resolved
.Select((state, cancellationToken) =>
.Select((state, _) =>
{
Debug.Assert(state.Item1 is not null);
object? result = GetRegexTypeToEmit(state.Item2, state.Item1, cancellationToken);
return result is RegexType regexType ? EmitRegexType(regexType, state.Item2) : result;
Debug.Assert(state.Left is not null);
return state.Left is RegexType regexType ? EmitRegexType(regexType, state.Right) : state.Left;
})
.Collect();

Expand Down Expand Up @@ -83,21 +82,5 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
context.AddSource("RegexGenerator.g.cs", string.Join(Environment.NewLine, code));
});
}

private sealed class LambdaComparer<T> : IEqualityComparer<T>
{
private readonly Func<T?, T?, bool> _equal;
private readonly Func<T?, int> _getHashCode;

public LambdaComparer(Func<T?, T?, bool> equal, Func<T?, int> getHashCode)
{
_equal = equal;
_getHashCode = getHashCode;
}

public bool Equals(T? x, T? y) => _equal(x, y);

public int GetHashCode(T obj) => _getHashCode(obj);
}
}
}
5 changes: 0 additions & 5 deletions src/libraries/System.Text.RegularExpressions/gen/Stubs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,3 @@ public RegexReplacement(string rep, RegexNode concat, Hashtable caps) { }
public const int WholeString = -4;
}
}

namespace System.Runtime.CompilerServices
{
internal static class IsExternalInit { }
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

<ItemGroup>
<!-- Common generator support -->
<Compile Include="$(CommonPath)Roslyn\GetBestTypeByMetadataName.cs" Link="Common\Roslyn\GetBestTypeByMetadataName.cs" />
<Compile Include="$(CommonPath)System\Runtime\CompilerServices\IsExternalInit.cs" Link="Common\System\Runtime\CompilerServices\IsExternalInit.cs" />

<!-- Code included from System.Text.RegularExpressions -->
<Compile Include="$(CommonPath)System\HexConverter.cs" Link="Production\HexConverter.cs" />
Expand Down