Allow RuntimeType with missing EEType in some cases #965
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
This is a different take on dotnet#620. We want to make sure `MakeArrayType`/`MakeGenericType` fails if we don't have an EEType for the constructed type because that's the problematic API and it's good UX to have it fail there (as opposed to failing later because e.g. we can't get a `TypeHandle` in a `CreateInstance` call). But it's really inconvenient to have this fail e.g. when we're hydrating types for the purpose of comparing method signatures. I've seen a failure yesterday where we couldn't find the `string..ctor(char[])` overload because we didn't have an EEType for `ReadOnlySpan<char>` (in a signature of an unrelated overload). The AOT compiler could create those EETypes, but it's very wasteful. Instead, I'm relaxing the EEType check so that we only do it from the public APIs. It means people can get to "unusable" `RuntimeTypes` now if they're e.g. field types, or parameter types, but I believe those will mostly be corner cases. I'm adding tests to make sure all of this is diagnosable.
MichalStrehovsky
added
the
area-NativeAOT-coreclr
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.
This is a different take on #620.
We want to make sure
MakeArrayType/MakeGenericTypefails if we don't have an EEType for the constructed type because that's the problematic API and it's good UX to have it fail there (as opposed to failing later because e.g. we can't get aTypeHandlein aCreateInstancecall).But it's really inconvenient to have this fail e.g. when we're hydrating types for the purpose of comparing method signatures. I've seen a failure yesterday where we couldn't find the
string..ctor(char[])overload because we didn't have an EEType forReadOnlySpan<char>(in a signature of an unrelated overload).The AOT compiler could create those EETypes, but it's very wasteful. Instead, I'm relaxing the EEType check so that we only do it from the public APIs. It means people can get to "unusable"
RuntimeTypesnow if they're e.g. field types, or parameter types, but I believe those will mostly be corner cases.I'm adding tests to make sure all of this is diagnosable.