-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix Type System Equivalence Checks #106498
Conversation
src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.TypeEquivalence.cs
Outdated
Show resolved
Hide resolved
src/coreclr/tools/Common/TypeSystem/Ecma/EcmaType.TypeEquivalence.cs
Outdated
Show resolved
Hide resolved
@davidwrighton do you see any harm in backporting this change to net8? |
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Failures are known and unrelated. Could we get a review now? @davidwrighton @MichalStrehovsky @lambdageek @kg @steveisok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Only outstanding question I have is if this is backport worthy?
I think this is eligible for backport. It looks reasonable to me. |
Perfect thanks! Will merge this and start working on the backport PR to .NET 8. |
/backport to release/9.0-rc1 |
Started backporting to release/9.0-rc1: https://github.com/dotnet/runtime/actions/runs/10461925315 |
/backport to release/8.0-staging |
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/10461927413 |
/backport to release/9.0 |
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10477927221 |
Fixes issue #96242.
This is the initial draft of the solution to address the aforementioned issue. Debugging through Crossgen2 and the compiler for ReadyToRun, we found that on Windows, we were computing and using type identifiers from Guid's, whether there is interop or not. As far as we're concerned, outside of those scenarios we shouldn't need to be doing those checks.
So, initially I want to ask for your guidance more than direct review feedback. Are we approaching this the right way? Is this the right solution? Starting this as draft since this discussion is needed beforehand.
@davidwrighton @jkotas @MichalStrehovsky