Skip to content

Mark Regex source generator Go override as SkipLocalsInit if possible #63277

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 3, 2022
Merged

Mark Regex source generator Go override as SkipLocalsInit if possible #63277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.Emitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public partial class RegexGenerator
};

/// <summary>Generates the code for one regular expression class.</summary>
private static (string, ImmutableArray<Diagnostic>) EmitRegexType(RegexType regexClass)
private static (string, ImmutableArray<Diagnostic>) EmitRegexType(RegexType regexClass, Compilation compilation)
{
var sb = new StringBuilder(1024);
var writer = new IndentedTextWriter(new StringWriter(sb));
Expand Down Expand Up @@ -82,7 +82,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);
ImmutableArray<Diagnostic> diagnostics = EmitRegexMethod(writer, regexClass.Method, generatedName, compilation);

while (writer.Indent != 0)
{
Expand Down Expand Up @@ -149,7 +149,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)
private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter writer, RegexMethod rm, string id, Compilation compilation)
{
string patternExpression = Literal(rm.Pattern);
string optionsExpression = Literal(rm.Options);
Expand All @@ -174,6 +174,8 @@ 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 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if all we use from compilation is this check, is it better to just pass the bool from the generator or do you expect us to use the Compilation object for something else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we're passing something, seemed like we might as well pass around the Compilation in case we need anything more from it later.


writer.WriteLine($"new {id}();");
writer.WriteLine();
writer.WriteLine($" private {id}()");
Expand Down Expand Up @@ -231,6 +233,10 @@ private static ImmutableArray<Diagnostic> EmitRegexMethod(IndentedTextWriter wri
writer.Indent -= 4;
writer.WriteLine($" }}");
writer.WriteLine();
if (allowUnsafe)
{
writer.WriteLine($" [global::System.Runtime.CompilerServices.SkipLocalsInit]");
}
writer.WriteLine($" protected override void Go()");
writer.WriteLine($" {{");
writer.Indent += 4;
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Text.RegularExpressions/gen/RegexGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
{
Debug.Assert(state.Item1 is not null);
object? result = GetRegexTypeToEmit(state.Item2, state.Item1, cancellationToken);
return result is RegexType regexType ? EmitRegexType(regexType) : result;
return result is RegexType regexType ? EmitRegexType(regexType, state.Item2) : result;
})
.Collect();

Expand Down
12 changes: 7 additions & 5 deletions ...ssions/tests/System.Text.RegularExpressions.Generators.Tests/RegexGeneratorParserTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,10 @@ partial class C
", compile: true));
}

[Fact]
public async Task Valid_ClassWithNamespace()
[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task Valid_ClassWithNamespace(bool allowUnsafe)
{
Assert.Empty(await RunGenerator(@"
using System.Text.RegularExpressions;
Expand All @@ -310,7 +312,7 @@ partial class C
private static partial Regex Valid();
}
}
", compile: true));
", compile: true, allowUnsafe: allowUnsafe));
}

[Fact]
Expand Down Expand Up @@ -557,13 +559,13 @@ partial class C
}

private async Task<IReadOnlyList<Diagnostic>> RunGenerator(
string code, bool compile = false, LanguageVersion langVersion = LanguageVersion.Preview, MetadataReference[]? additionalRefs = null, CancellationToken cancellationToken = default)
string code, bool compile = false, LanguageVersion langVersion = LanguageVersion.Preview, MetadataReference[]? additionalRefs = null, bool allowUnsafe = false, CancellationToken cancellationToken = default)
{
var proj = new AdhocWorkspace()
.AddSolution(SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create()))
.AddProject("RegexGeneratorTest", "RegexGeneratorTest.dll", "C#")
.WithMetadataReferences(additionalRefs is not null ? s_refs.Concat(additionalRefs) : s_refs)
.WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)
.WithCompilationOptions(new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: allowUnsafe)
.WithNullableContextOptions(NullableContextOptions.Enable))
.WithParseOptions(new CSharpParseOptions(langVersion))
.AddDocument("RegexGenerator.g.cs", SourceText.From(code, Encoding.UTF8)).Project;
Expand Down