Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

Fixes #107541.

Cc @dotnet/ilc-contrib

@neon-sunset
Copy link
Contributor

Thank you!

Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

A few suggestions on how you could use patterns to remove some repeated conditionals.

Feel free to ignore this feedback. Sometimes I find this useful, sometimes it reads worse. It's a judgement call.


instance = new ReadOnlySpanValue(readOnlySpanElementType, bytes, byteOffset, byteLength);
}
else if (readOnlySpanElementType != null && ctorSig.Length == 1 && ctorSig[0].IsArray
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
else if (readOnlySpanElementType != null && ctorSig.Length == 1 && ctorSig[0].IsArray
else if (readOnlySpanElementType != null && ctorSig is [ { IsArray: true }]

@MichalStrehovsky MichalStrehovsky merged commit bb9cd26 into dotnet:main Sep 16, 2024
@MichalStrehovsky MichalStrehovsky deleted the fix107541 branch September 16, 2024 06:11
@am11
Copy link
Member

am11 commented Sep 16, 2024

Not a regression but could it be a 9.0 candidate?

@MichalStrehovsky
Copy link
Member Author

Not a regression but could it be a 9.0 candidate?

We have a formal list of things that are allowed into 9.0 at this point and the only perf fixes that would meet 9.0 bar are "Significant performance regression from last release" (not just regression, it would also need to be significant to even consider a backport).

jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2024
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.

[NativeAOT] Static constructor interpreter does not support spans

5 participants