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

Warnings for in/readonly ref #77625

Open
Tracked by #79053
benaadams opened this issue Nov 21, 2017 · 9 comments
Open
Tracked by #79053

Warnings for in/readonly ref #77625

benaadams opened this issue Nov 21, 2017 · 9 comments
Labels
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 code-fixer Marks an issue that suggests a Roslyn code fixer
Milestone

Comments

@benaadams
Copy link
Member

benaadams commented Nov 21, 2017

Primitive value-types that will be passed by register (byte, short, int, etc) should generate a warning when passed as in as passing by readonly reference in the common case is likely a deoptimization error (dereference vs register)

Reference-types should generate a warning when passed as in as passing by readonly reference in the common case is likely a deoptimization error (double dereference vs dereference)

from: dotnet/roslyn#23327

/cc @mikedn

@jcouv
Copy link
Member

jcouv commented Nov 22, 2017

Tagging @VSadov

@VSadov
Copy link
Member

VSadov commented Nov 22, 2017

There are cases where having an in parameter typed as reference type makes sense.
Like Volatile.Read(in object variable)

In general it is indeed not very useful, but there would be a need for a whitelist of few reasonable uses.
I could be a good idea for an analyzer.

@benaadams
Copy link
Member Author

There are cases where having an in parameter typed as reference type makes sense.
Like Volatile.Read(in object variable)

But they could be #pragma'd as valid?

@tannergooding
Copy link
Member

I agree this would be a good case for an analyzer.

That being said, for some types whether or not it should be passed as 'in' depends on the underlying architecture and calling convention. So the diagnostics should likely only cover the well-defined cases that are consistent between all targets.

@MaStr11
Copy link

MaStr11 commented Dec 20, 2017

This should be considered for the warning waves dotnet/roslyn#1580 and the code fix candidates dotnet/roslyn#23326.

@CyrusNajmabadi CyrusNajmabadi transferred this issue from dotnet/roslyn Oct 29, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 29, 2022
@mavasani mavasani transferred this issue from dotnet/roslyn-analyzers Oct 29, 2022
@mavasani
Copy link

Transferred to dotnet/runtime for triage

@ghost
Copy link

ghost commented Oct 30, 2022

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

Issue Details

Primitive value-types that will be passed by register (byte, short, int, etc) should generate a warning when passed as in as passing by readonly reference in the common case is likely a deoptimization error (dereference vs register)

Reference-types should generate a warning when passed as in as passing by readonly reference in the common case is likely a deoptimization error (double dereference vs dereference)

from: dotnet/roslyn#23327

/cc @mikedn

Author: benaadams
Assignees: -
Labels:

area-System.Runtime, untriaged

Milestone: -

@stephentoub stephentoub added code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer labels Oct 31, 2022
@buyaa-n
Copy link
Contributor

buyaa-n commented Oct 31, 2022

So the diagnostics should likely only cover the well-defined cases that are consistent between all targets.

A list of cases that need to be covered would be very helpful for further triaging and implementing the analyzer.

For reference types it is suggested to exclude object type. What cases should be excluded from primitive value-types that will be passed as in parameter?

Please comment any exclusions should be covered

@buyaa-n buyaa-n added this to the Future milestone Oct 31, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 31, 2022
@buyaa-n buyaa-n modified the milestones: Future, 8.0.0 Nov 11, 2022
@buyaa-n buyaa-n added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 19, 2022
@tannergooding tannergooding modified the milestones: 8.0.0, Future Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 code-fixer Marks an issue that suggests a Roslyn code fixer
Projects
None yet
Development

No branches or pull requests

9 participants