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

Analyzer suggestion: unnecessary use of unsafe #48031

Open
stephentoub opened this issue Sep 24, 2020 · 4 comments
Open

Analyzer suggestion: unnecessary use of unsafe #48031

stephentoub opened this issue Sep 24, 2020 · 4 comments
Labels
Area-Compilers Feature Request IDE-CodeStyle Built-in analyzers, fixes, and refactorings
Milestone

Comments

@stephentoub
Copy link
Member

Brief description:

We frequently find ourselves initially writing some method to use unsafe code / pointers, and so annotate the method as unsafe. Later, generally as Span<T> is employed more or as new framework APIs are utilized to improve the method, the method no longer uses pointers, but we sometimes forget to remove the unsafe keyword. It would be nice-to-have if there were an analyzer we could enable to flag such uses of unsafe that could be removed.

Languages applicable:

C#

Code example that the analyzer should report:

private unsafe void ToUpper(Span<char> span)
{
    for (int i = 0; i < span.Length; i++)
        span[i] = char.ToUpperInvariant(span[i]);
}
@sharwell sharwell added Area-IDE Feature Request IDE-CodeStyle Built-in analyzers, fixes, and refactorings Need Design Review The end user experience design needs to be reviewed and approved. labels Sep 24, 2020
@sharwell sharwell added this to In Queue in IDE: Design review via automation Sep 24, 2020
@sharwell sharwell moved this from In Queue to On deck in IDE: Design review Sep 24, 2020
@sharwell
Copy link
Member

sharwell commented Sep 24, 2020

I've wanted this for sure. Also have wanted the ability to move unsafe from a type declaration to a subset of members that actually need it.

@Youssef1313
Copy link
Member

I'm interested in implementing this if it's approved in the design meeting.

@CyrusNajmabadi
Copy link
Member

I'm willing to pre-emptively approve this. We would take this. Open question is on if it's feasible to encode the rules easily enough to put this in teh IDE. My preference would be more that this be done in the compiler with a hidden diagnostic.

@sharwell sharwell moved this from On deck to Next meeting in IDE: Design review Oct 5, 2020
@sharwell sharwell moved this from Next meeting to Need Update in IDE: Design review Oct 5, 2020
@jinujoseph jinujoseph added this to the Backlog milestone Oct 15, 2020
@vatsalyaagrawal
Copy link
Contributor

Design meeting notes: This should come from Compiler, request hidden diagnostic from the compiler.

@vatsalyaagrawal vatsalyaagrawal removed the Need Design Review The end user experience design needs to be reviewed and approved. label Oct 18, 2020
@vatsalyaagrawal vatsalyaagrawal moved this from Need Update to Complete in IDE: Design review Oct 18, 2020
stephentoub referenced this issue in dotnet/runtime Aug 17, 2023
Simplifies BitHelper, basing it on span instead of having to accomodate both pointers and arrays, and avoids allocating the BitHelper instance.  We now also clear the input optionally.


Commit migrated from dotnet/corefx@82ce02d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature Request IDE-CodeStyle Built-in analyzers, fixes, and refactorings
Projects
Status: Complete
IDE: Design review
  
Complete
Development

No branches or pull requests

6 participants