From 10701a9c30e1458251971d513968cf583e9efaf0 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 8 Feb 2022 19:38:26 +0200 Subject: [PATCH 1/4] Optimize the Regex source generator's handling of Compilation objects. --- .../gen/RegexGenerator.Emitter.cs | 8 ++-- .../gen/RegexGenerator.Parser.cs | 28 +++++------- .../gen/RegexGenerator.cs | 45 ++++++------------- 3 files changed, 29 insertions(+), 52 deletions(-) diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs index 1f2a5d7de00a03..c2c9908398848e 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs @@ -37,7 +37,7 @@ public partial class RegexGenerator }; /// Generates the code for one regular expression class. - private static (string, ImmutableArray) EmitRegexType(RegexType regexClass, Compilation compilation) + private static (string, ImmutableArray) EmitRegexType(RegexType regexClass, bool allowUnsafe) { var sb = new StringBuilder(1024); var writer = new IndentedTextWriter(new StringWriter(sb)); @@ -78,7 +78,7 @@ private static (string, ImmutableArray) EmitRegexType(RegexType rege generatedName += ComputeStringHash(generatedName).ToString("X"); // Generate the regex type - ImmutableArray diagnostics = EmitRegexMethod(writer, regexClass.Method, generatedName, compilation); + ImmutableArray diagnostics = EmitRegexMethod(writer, regexClass.Method, generatedName, allowUnsafe); while (writer.Indent != 0) { @@ -145,7 +145,7 @@ static bool ExceedsMaxDepthForSimpleCodeGeneration(RegexNode node, int allowedDe } /// Generates the code for a regular expression method. - private static ImmutableArray EmitRegexMethod(IndentedTextWriter writer, RegexMethod rm, string id, Compilation compilation) + private static ImmutableArray EmitRegexMethod(IndentedTextWriter writer, RegexMethod rm, string id, bool allowUnsafe) { string patternExpression = Literal(rm.Pattern); string optionsExpression = Literal(rm.Options); @@ -170,8 +170,6 @@ private static ImmutableArray 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}()"); diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs index a7ba253ee2c929..6dc28063309c33 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs @@ -21,37 +21,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) { - 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; } @@ -61,8 +59,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) { @@ -81,7 +77,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) { continue; } @@ -127,7 +123,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()); } diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs index 558c613eae3a8f..01778c832efbd0 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs @@ -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 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> 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.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 - .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(); @@ -83,21 +82,5 @@ public void Initialize(IncrementalGeneratorInitializationContext context) context.AddSource("RegexGenerator.g.cs", string.Join(Environment.NewLine, code)); }); } - - private sealed class LambdaComparer : IEqualityComparer - { - private readonly Func _equal; - private readonly Func _getHashCode; - - public LambdaComparer(Func equal, Func getHashCode) - { - _equal = equal; - _getHashCode = getHashCode; - } - - public bool Equals(T? x, T? y) => _equal(x, y); - - public int GetHashCode(T obj) => _getHashCode(obj); - } } } From 033879812f64696293698dfc12c72ef388150b39 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 8 Feb 2022 19:46:37 +0200 Subject: [PATCH 2/4] Use the common downlevel IsExternalInit file in the regex source generator. And remove a now-unused file. --- .../gen/RegexGenerator.Parser.cs | 1 - src/libraries/System.Text.RegularExpressions/gen/Stubs.cs | 5 ----- .../gen/System.Text.RegularExpressions.Generator.csproj | 2 +- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs index 6dc28063309c33..670122117ae5ef 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs @@ -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; diff --git a/src/libraries/System.Text.RegularExpressions/gen/Stubs.cs b/src/libraries/System.Text.RegularExpressions/gen/Stubs.cs index 13626a4be5a3bc..2930b513e66740 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/Stubs.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/Stubs.cs @@ -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 { } -} diff --git a/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj b/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj index e6982a3b83ce0e..e6d7c1274aad58 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj +++ b/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj @@ -22,7 +22,7 @@ - + From 00ad4be685f9abddcebd835cf0ce018e732668d8 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 21 Feb 2022 00:15:37 +0200 Subject: [PATCH 3/4] Address PR feedback; revert to the old way of matching symbols. --- .../gen/RegexGenerator.Parser.cs | 17 ++++++++++++++--- ...tem.Text.RegularExpressions.Generator.csproj | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs index 670122117ae5ef..5053b535ae64e6 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Parser.cs @@ -4,6 +4,7 @@ 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; @@ -44,14 +45,24 @@ private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, M // 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? GetSemanticTargetForGeneration(GeneratorSyntaxContext context, CancellationToken cancellationToken) { - SemanticModel sm = context.SemanticModel; var methodSyntax = (MethodDeclarationSyntax)context.Node; + SemanticModel sm = context.SemanticModel; if (!IsSemanticTargetForGeneration(sm, methodSyntax, cancellationToken)) { return null; } + Compilation compilation = sm.Compilation; + INamedTypeSymbol? regexSymbol = compilation.GetBestTypeByMetadataName(RegexName); + INamedTypeSymbol? regexGeneratorAttributeSymbol = compilation.GetBestTypeByMetadataName(RegexGeneratorAttributeName); + + if (regexSymbol is null || regexGeneratorAttributeSymbol is null) + { + // Required types aren't available + return null; + } + TypeDeclarationSyntax? typeDec = methodSyntax.Parent as TypeDeclarationSyntax; if (typeDec is null) { @@ -76,7 +87,7 @@ private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, M int? matchTimeout = null; foreach (AttributeData attributeData in boundAttributes) { - if (attributeData.AttributeClass?.ToDisplayString()?.Equals(RegexGeneratorAttributeName, StringComparison.Ordinal) != true) + if (attributeData.AttributeClass?.Equals(regexGeneratorAttributeSymbol) != true) { continue; } @@ -122,7 +133,7 @@ private static bool IsSemanticTargetForGeneration(SemanticModel semanticModel, M if (!regexMethodSymbol.IsPartialDefinition || regexMethodSymbol.Parameters.Length != 0 || regexMethodSymbol.Arity != 0 || - !regexMethodSymbol.ReturnType.ToDisplayString().Equals(RegexName, StringComparison.Ordinal)) + !regexMethodSymbol.ReturnType.Equals(regexSymbol)) { return Diagnostic.Create(DiagnosticDescriptors.RegexMethodMustHaveValidSignature, methodSyntax.GetLocation()); } diff --git a/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj b/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj index e6d7c1274aad58..594725eb0aa69b 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj +++ b/src/libraries/System.Text.RegularExpressions/gen/System.Text.RegularExpressions.Generator.csproj @@ -22,6 +22,7 @@ + From d9878b256638b746392301797aa7dffe950553e6 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Wed, 23 Feb 2022 10:47:20 +0200 Subject: [PATCH 4/4] Fix an outdated comment. --- .../System.Text.RegularExpressions/gen/RegexGenerator.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs index 01778c832efbd0..b840aff351e5a4 100644 --- a/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs +++ b/src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs @@ -44,7 +44,8 @@ public void Initialize(IncrementalGeneratorInitializationContext context) // Pair each with whether unsafe code is allowed .Combine(allowUnsafeProvider) - // Get the resulting code string or error Diagnostic for each MethodDeclarationSyntax/Compilation pair + // Get the resulting code string or error Diagnostic for + // each MethodDeclarationSyntax/allow-unsafe-blocks pair .Select((state, _) => { Debug.Assert(state.Left is not null);