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

Runtime type loader support for static abstract interface methods #67745

Closed
MichalStrehovsky opened this issue Apr 8, 2022 · 1 comment · Fixed by #71321
Closed

Runtime type loader support for static abstract interface methods #67745

MichalStrehovsky opened this issue Apr 8, 2022 · 1 comment · Fixed by #71321

Comments

@MichalStrehovsky
Copy link
Member

The compiler currently generates Invalid dictionary entries for constrained calls. So if we have static abstract methods used in code that got MakeGenericType'd/MakeGenerciMethod'ed at runtime, we'll get a MissingRuntimeArtifactException even if the type loader template was available for such type. The workaround is RD.XML the specific instantiation.

We should add type loader support for this.

@MichalStrehovsky MichalStrehovsky added this to the 7.0.0 milestone Apr 8, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 8, 2022
@MichalStrehovsky MichalStrehovsky removed the untriaged New issue has not been triaged by the area owner label Apr 8, 2022
@MichalStrehovsky
Copy link
Member Author

When closing this, make sure the repro from #70951 works.

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jun 22, 2022
When a generic dictionary has multiple constrained calls, we would crash the compiler.

I hit this in tests for dotnet#67745 but I want to separate this from the fix because that one is going to be quite a bit of code.
jkotas pushed a commit that referenced this issue Jun 22, 2022
When a generic dictionary has multiple constrained calls, we would crash the compiler.

I hit this in tests for #67745 but I want to separate this from the fix because that one is going to be quite a bit of code.
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jun 27, 2022
Fixes dotnet#67745.

Support for static virtual methods that was added in dotnet#66084 was enough for compile-time resolution of static virtual methods, but didn't cover dynamic code. For example, given following code:

```csharp
interface IFoo { static virtual void Frob(); }
class SomeCaller<T> where T : IFoo { ... T.Frob(); }
class SomeClass : IFoo { ... }
```

If we do `typeof(SomeCaller<>).MakeGenericType(typeof(SomeClass)` at runtime, the runtime has to find what method implements `IFoo.Frob` on `SomeClass` and ensure proper data structures are generated for `SomeCaller<SomeClass>` so that the call lands in the right spot at runtime.

On a high level, what we need:
* Change to the compiler to generate extra data ("interface dispatch maps") that lets us find an implementation of interface method X on a given type Y.
* Change to the runtime to read the new data structure.
* Change to the compiler to generate extra method bodies for types that can potentially be used with MakeGeneric at runtime. This is an overapproximation since we don't know the set of types that will really be used.
* Change to type loader data structures to capture when shared generic code needs to do this mapping, and change the code in the compiler that emits it, and to the type loader that reads it.

I've made it so that the dispatch logic between instance and static methods is shared. It's not strictly necessary for both to go into the same data structure, but it prevents duplicating the code on the emission and reading side. The side effect of that is that static virtual methods now go into the sealed vtable. We have to put them somewhere. This spot is as good as any.

I've also had to make a small change to the ordering of data structure generation within the type loader. I've made is so that EEType/MethodTable structures are fully populated before we start filling out generic dictionaries. This prevents us from calling into the runtime dispatch logic with EETypes/MethodTables that are not actually built yet. I really didn't want to duplicate the dispatch logic into the type loader.
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 27, 2022
MichalStrehovsky added a commit that referenced this issue Jun 28, 2022
Fixes #67745.

Support for static virtual methods that was added in #66084 was enough for compile-time resolution of static virtual methods, but didn't cover dynamic code. For example, given following code:

```csharp
interface IFoo { static virtual void Frob(); }
class SomeCaller<T> where T : IFoo { ... T.Frob(); }
class SomeClass : IFoo { ... }
```

If we do `typeof(SomeCaller<>).MakeGenericType(typeof(SomeClass)` at runtime, the runtime has to find what method implements `IFoo.Frob` on `SomeClass` and ensure proper data structures are generated for `SomeCaller<SomeClass>` so that the call lands in the right spot at runtime.

On a high level, what we need:
* Change to the compiler to generate extra data ("interface dispatch maps") that lets us find an implementation of interface method X on a given type Y.
* Change to the runtime to read the new data structure.
* Change to the compiler to generate extra method bodies for types that can potentially be used with MakeGeneric at runtime. This is an overapproximation since we don't know the set of types that will really be used.
* Change to type loader data structures to capture when shared generic code needs to do this mapping, and change the code in the compiler that emits it, and to the type loader that reads it.

I've made it so that the dispatch logic between instance and static methods is shared. It's not strictly necessary for both to go into the same data structure, but it prevents duplicating the code on the emission and reading side. The side effect of that is that static virtual methods now go into the sealed vtable. We have to put them somewhere. This spot is as good as any.

I've also had to make a small change to the ordering of data structure generation within the type loader. I've made it so that EEType/MethodTable structures are fully populated before we start filling out generic dictionaries. This prevents us from calling into the runtime dispatch logic with EETypes/MethodTables that are not actually built yet. I really didn't want to duplicate the dispatch logic into the type loader.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 28, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant