Skip to content

Use Type.IsByRefLike. #48351

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 2 commits into from
May 24, 2023
Merged

Use Type.IsByRefLike. #48351

merged 2 commits into from
May 24, 2023

Conversation

teo-tsirpanis
Copy link
Contributor

Use Type.IsByRefLike.

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Use the Type.IsByRefLike property to check if a type is a ref struct.

Description

In PropertyHelper.cs we check if the property's type is a ref struct by checking if it is a value type and has the System.Runtime.CompilerServices.IsByRefLikeAttribute applied to it. This is inefficient and unreliable in edge cases (such as a ref struct defined in an assembly targeting .NET Standard).

This PR changes the code to just call property.PropertyType.IsByRefLike, which is more efficient and returns the authoritative answer on whether the runtime considers a type to be a ref struct. RequestBuilder.cs already uses this property.

PropertyHelpers.cs is used only on projects targeting modern frameworks so there is no need to #if the change.

@ghost ghost added needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically community-contribution Indicates that the PR has been added by a community member labels May 21, 2023
@ghost
Copy link

ghost commented May 21, 2023

Thanks for your PR, @teo-tsirpanis. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@teo-tsirpanis teo-tsirpanis changed the title Isbyreflike Use Type.IsByRefLike. May 21, 2023
@mkArtakMSFT mkArtakMSFT added area-runtime and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels May 22, 2023
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

Seems like a nice simplification - thanks!

@amcasey amcasey merged commit 91d702c into dotnet:main May 24, 2023
@teo-tsirpanis teo-tsirpanis deleted the isbyreflike branch May 24, 2023 16:12
@ghost ghost added this to the 8.0-preview6 milestone May 24, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants