Initial default interface method support #368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Support for default interface methods in the compiler and runtime. It felt like the most straightforward approach is to just give interface types a dispatch map and populate vtables of interface method with the default implementation. I didn't realize they get a full vtable with a bunch of null slots. This might be something we could potentially optimize, but it's orthogonal. There's plenty of things missing still, but it's a start and tests are starting to pass.
MichalStrehovsky
added
the
area-NativeAOT-coreclr
|
What is the type loader behavior when it hits a type with DIMs? It would be nice if it was a clean crash, and not a bad AV somewhere. |
The type loader is not involved right now since I'm not attempting to solve the "we need an instantiating stub for shared methods on generic interfaces". Right now we would just end up calling a shared method without its generic context. I pushed out an improvement that treats these as a reabstraction instead (so we never end up placing the canonical method without a generic context into the vtable). I also added a couple very basic tests so that we can smoke test this. We should definitely do a Pri-0 test pass before merging. |
|
/azp run runtimelab-nativeaot Pri-0 Tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtimelab-nativeaot Pri-0 Tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
The StaticInterfaceMembers failure was also present in the https://github.com/dotnet/runtimelab/runs/1405225153 run so it's unrelated. I'll take a look when we clean up the Pri-0 runs again. I couldn't get it to repro in single file, so it's probably a multifile specific wart. |
MichalStrehovsky
added a commit
to MichalStrehovsky/runtimelab
that referenced
this pull request
May 11, 2021
This reverts commit 6898730.
MichalStrehovsky
added a commit
to MichalStrehovsky/runtimelab
that referenced
this pull request
May 11, 2021
Different take from dotnet#368 - this time we're adding extra slots into the (sealed) vtable of the implementing class and pointing the dispatch map of the owning type to those slots when a default implementation is needed. This strategy has a couple advantages: * Main motivation: it will make implementing canonical interface methods possible - we will have a good spot to grab the interface type (instantiation argument of the default interface method) from, since we can now have per-implementing-type entrypoints. * We are now able to figure out a default implementation is not needed at compile time (we don't generate a body unless there's a class that needs it) * Dispatch is pre-computed (no longer have to hunt for the default implementation in dispatch maps of the interfaces at runtime). The disadvantage is that this strategy is a bit more complicated, but it doesn't actually look too terrible.
MichalStrehovsky
added a commit
to MichalStrehovsky/runtimelab
that referenced
this pull request
May 11, 2021
Different take from dotnet#368 - this time we're adding extra slots into the (sealed) vtable of the implementing class and pointing the dispatch map of the owning type to those slots when a default implementation is needed. This strategy has a couple advantages: * Main motivation: it will make implementing canonical interface methods possible - we will have a good spot to grab the interface type (instantiation argument of the default interface method) from, since we can now have per-implementing-type entrypoints. * We are now able to figure out a default implementation is not needed at compile time (we don't generate a body unless there's a class that needs it) * Dispatch is pre-computed (no longer have to hunt for the default implementation in dispatch maps of the interfaces at runtime). The disadvantage is that this strategy is a bit more complicated, but it doesn't actually look too terrible.
jkotas
pushed a commit
that referenced
this pull request
May 11, 2021
* Revert "Initial default interface method support (#368)" This reverts commit 6898730. * Add back changes we want to keep * Implement support for default interface methods Different take from #368 - this time we're adding extra slots into the (sealed) vtable of the implementing class and pointing the dispatch map of the owning type to those slots when a default implementation is needed. This strategy has a couple advantages: * Main motivation: it will make implementing canonical interface methods possible - we will have a good spot to grab the interface type (instantiation argument of the default interface method) from, since we can now have per-implementing-type entrypoints. * We are now able to figure out a default implementation is not needed at compile time (we don't generate a body unless there's a class that needs it) * Dispatch is pre-computed (no longer have to hunt for the default implementation in dispatch maps of the interfaces at runtime). The disadvantage is that this strategy is a bit more complicated, but it doesn't actually look too terrible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for default interface methods in the compiler and runtime.
It felt like the most straightforward approach is to just give interface types a dispatch map and populate vtables of interface method with the default implementation. I didn't realize they get a full vtable with a bunch of null slots. This might be something we could potentially optimize, but it's orthogonal.
There's plenty of things missing still, but it's a start and tests are starting to pass.