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

Add default method support to virtual statics #64717

Conversation

davidwrighton
Copy link
Member

  • The preview feature version of virtual statics implemented for .NET 6 does not allow for the interface methods to have a default implementation.
  • With this change, we add support for the interface method having an actual implementation to CoreCLR. From what I can tell the Mono runtime already had such support
  • There are some small ECMA spec updates to allow this change that are also included

In addition, I've taken the liberty to enable running the coreclr test suite on Mono on Windows. It needed a small amount of fixup.

@davidwrighton
Copy link
Member Author

@vargaz When I run the new tests on Mono, they fail with crashes after 12 tests. However, we ALSO fail with a crash after 12 tests in the old test which hasn't been changed. When do we plan to fix this test suite for Mono?
@MichalStrehovsky I have not updated anything in NativeAOT. Have static virtuals been implemented for NativeAOT yet or are you waiting for the Crossgen2 implementation to be implemented?
@trylek fyi

@MichalStrehovsky
Copy link
Member

@MichalStrehovsky I have not updated anything in NativeAOT. Have static virtuals been implemented for NativeAOT yet or are you waiting for the Crossgen2 implementation to be implemented?

I've been waiting for the type system change part of #54063 but since it looks like it might not land anytime soon, I might just take the type system part, finish it, and run with it. Might not get to that soon either though. But it looks like NativeAOT has more urgency for that for .NET 7 than crossgen2 has.

@davidwrighton davidwrighton requested a review from trylek February 3, 2022 19:57
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

This is indeed very nice and clean, thanks David! The expanded test set will be of immense help when stabilizing SVM resolution in Crossgen2 in the presence of default interface methods.

@davidwrighton
Copy link
Member Author

/azp list

@azure-pipelines
Copy link

@davidwrighton
Copy link
Member Author

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidwrighton
Copy link
Member Author

Running an outerloop crossgen test to make sure that the rather long new test names don't cause significant issues in our test suite.

@davidwrighton
Copy link
Member Author

/azp run runtime-coreclr crossgen2 outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@marek-safar
Copy link
Contributor

/cc @SamMonoRT

@davidwrighton davidwrighton merged commit 9e45de4 into dotnet:main Feb 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 11, 2022
@davidwrighton davidwrighton deleted the add_default_method_support_to_virtual_statics branch April 13, 2023 18:50
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.

4 participants