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

Extract array of const to static readonly field #33794

Closed
Tracked by #57797 ...
terrajobst opened this issue Mar 19, 2020 · 6 comments · Fixed by dotnet/roslyn-analyzers#5383
Closed
Tracked by #57797 ...

Extract array of const to static readonly field #33794

terrajobst opened this issue Mar 19, 2020 · 6 comments · Fixed by dotnet/roslyn-analyzers#5383
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@terrajobst
Copy link
Member

Arrays of const passed to known methods on types like System.String (e.g. IndexOfAny(new[] { ',', '.' })) can be lifted out to static readonly fields.

Category: Performance

@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer labels Mar 19, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 19, 2020
@jeffhandley jeffhandley added this to the 5.0 milestone Mar 20, 2020
@jeffhandley jeffhandley added the code-fixer Marks an issue that suggests a Roslyn code fixer label Mar 21, 2020
@jeffhandley
Copy link
Member

Estimates:

  • Analyzer: Small
  • Fixer: Large

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Mar 21, 2020
@jeffhandley
Copy link
Member

Moving this one out of .NET 5, but this would be a good warmup analyzer for someone, and it would be a high-value analyzer in .NET 6 because it applies to some recently added APIs.

@jeffhandley jeffhandley added the good first issue Issue should be easy to implement, good for first-time contributors label May 26, 2020
@jeffhandley jeffhandley modified the milestones: 5.0, Future May 26, 2020
@jeffhandley
Copy link
Member

The hardest part of this one will be coming up with the list of APIs we want this to target--using a heuristic is the recommended approach. Beyond that, we think the analyzer will be pretty easy; the fixer will have some edge cases to consider for how to rewrite the code to reflect the definition/usage of the const--simply extracting the const won't necessarily be straightforward, and there will need to be an ordered list of approaches to apply.

@jakub-klima
Copy link

Cannot optimization like this be done by the compiler?

@stephentoub stephentoub removed the good first issue Issue should be easy to implement, good for first-time contributors label Sep 26, 2020
@stephentoub stephentoub added the help wanted [up-for-grabs] Good issue for external contributors label Oct 5, 2020
@buyaa-n buyaa-n modified the milestones: Future, 6.0.0 Oct 30, 2020
@teo-tsirpanis
Copy link
Contributor

Such optimization could be automatically done by the compiler for methods that accept a ReadOnlySpan.

@tannergooding tannergooding modified the milestones: 6.0.0, Future Jul 12, 2021
@steveberdy
Copy link
Contributor

@buyaa-n I would like this assigned to me. Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants