Skip to content

Add support for static default interface methods#72661

Merged
MichalStrehovsky merged 2 commits intodotnet:mainfrom
MichalStrehovsky:statdefltintf
Jul 22, 2022
Merged

Add support for static default interface methods#72661
MichalStrehovsky merged 2 commits intodotnet:mainfrom
MichalStrehovsky:statdefltintf

Conversation

@MichalStrehovsky
Copy link
Copy Markdown
Member

We were only supporting default interface methods in the sense of non-abstract virtuals. We now also support overrides.

What's missing is support for diamond/reabstraction. I think we need to make throwing stubs that we can dispatch to - it doesn't look like the strategy used in the VM (callsiteCalloutHelper) can be used for prejit. I don't want to increase the scope of this pull request, so I currently just invalidate the entire method that has such dispatch.

Cc @dotnet/ilc-contrib

Fixes #70930.

We were only supporting default interface methods in the sense of non-abstract virtuals. We now also support overrides.

What's missing is support for diamond/reabstraction. I thin we need to make throwing stubs that we can dispatch to - it doesn't look like the strategy used in the VM (`callsiteCalloutHelper`) can be used for prejit. I don't want to increase the scope of this pull request, so we invalidate the entire method that has such dispatch.
Comment on lines +605 to +607
// It's difficult to discern what runtime determined form the interface method
// is on later so fail the resolution if this would be that.
// This is pretty conservative and can be narrowed down.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I went down this rabbit hole and it lead to stuff like:

https://github.com/MichalStrehovsky/runtime/blob/c7e5c934159fc27b5c92a0c609a6756cf43a26fe/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Compilation.cs#L519-L622

I really didn't like it and had trouble getting it to work. The problem is that RyuJIT doesn't operate on runtime-determined things and we can't even properly resolve such constrained calls (the support for that was never needed in the type system because only universal shared code needs to resolve constrained calls on runtime determined types otherwise and that was always supported by the STS type system in NUTC).

// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this using really needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup, Unsafe.NullRef so that users who don't need static method support av if they accidentally pass static methods.

Copy link
Copy Markdown
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@MichalStrehovsky MichalStrehovsky merged commit fce6549 into dotnet:main Jul 22, 2022
@MichalStrehovsky MichalStrehovsky deleted the statdefltintf branch July 22, 2022 21:32
@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2022
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: Failed to compile virtual statics with default implementation

2 participants