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.

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

Conversation

stephentoub
Copy link
Member

It can have so many locals that zero-initing is measurable.

It can have so many locals that zero-initing is measurable.
@stephentoub stephentoub added this to the 7.0.0 milestone Jan 3, 2022
@ghost ghost assigned stephentoub Jan 3, 2022
@ghost
Copy link

ghost commented Jan 3, 2022

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

It can have so many locals that zero-initing is measurable.

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions, tenet-performance

Milestone: 7.0.0

@@ -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.

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

LGTM. Isn't this something that the Compiled engine can benefit from?

@stephentoub
Copy link
Member Author

Isn't this something that the Compiled engine can benefit from?

I don't believe it's relevant to dynamic methods.

@stephentoub stephentoub merged commit 6c50d9f into dotnet:main Jan 3, 2022
@stephentoub stephentoub deleted the markgoasskiplocalsinit branch January 3, 2022 18:32
@ghost ghost locked as resolved and limited conversation to collaborators Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants