-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Crossgen2 support for static virtual method resolution #54063
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
Changes from all commits
bbeb1f3
6191fa3
d5cd7a4
5eea0c5
b4e78cf
118e3fa
91e1805
9d5c7f5
c3248ec
147bf51
4815ff7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -572,6 +572,11 @@ public override MethodDesc ResolveVariantInterfaceMethodToVirtualMethodOnType(Me | |||||
| return ResolveVariantInterfaceMethodToVirtualMethodOnType(interfaceMethod, (MetadataType)currentType); | ||||||
| } | ||||||
|
|
||||||
| public override MethodDesc ResolveVariantInterfaceMethodToStaticVirtualMethodOnType(MethodDesc interfaceMethod, TypeDesc currentType, Func<TypeDesc, bool> inVersionBubble) | ||||||
| { | ||||||
| return ResolveVariantInterfaceMethodToStaticVirtualMethodOnType(interfaceMethod, (MetadataType)currentType, inVersionBubble); | ||||||
| } | ||||||
|
|
||||||
| //////////////////////// INTERFACE RESOLUTION | ||||||
| //Interface function resolution | ||||||
| // Interface function resolution follows the following rules | ||||||
|
|
@@ -826,5 +831,125 @@ public static IEnumerable<MethodDesc> EnumAllVirtualSlots(MetadataType type) | |||||
| } while (type != null); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Try to resolve a given virtual static interface method on a given constrained type and its base types. | ||||||
| /// </summary> | ||||||
| /// <param name="interfaceMethod">Interface method to resolve</param> | ||||||
| /// <param name="currentType">Type to attempt virtual static method resolution on</param> | ||||||
| /// <returns>MethodDesc of the resolved virtual static method, null when not found (runtime lookup must be used)</returns> | ||||||
| public static MethodDesc ResolveVariantInterfaceMethodToStaticVirtualMethodOnType(MethodDesc interfaceMethod, MetadataType currentType, Func<TypeDesc, bool> inVersionBubble) | ||||||
| { | ||||||
| TypeDesc interfaceType = interfaceMethod.OwningType; | ||||||
| if (!inVersionBubble(interfaceType)) | ||||||
| { | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| // Search for match on a per-level in the type hierarchy | ||||||
| for (TypeDesc typeToCheck = currentType; typeToCheck != null; typeToCheck = typeToCheck.BaseType) | ||||||
| { | ||||||
| if (!inVersionBubble(typeToCheck)) | ||||||
| { | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| MethodDesc resolvedMethodOnType = TryResolveVirtualStaticMethodOnThisType(typeToCheck, interfaceMethod); | ||||||
| if (resolvedMethodOnType != null) | ||||||
| { | ||||||
| return resolvedMethodOnType; | ||||||
| } | ||||||
|
|
||||||
| // Variant interface dispatch | ||||||
|
||||||
| foreach (DefType runtimeInterfaceType in typeToCheck.RuntimeInterfaces) | ||||||
| { | ||||||
| if (runtimeInterfaceType == interfaceType) | ||||||
| { | ||||||
| // This is the variant interface check logic, skip this | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| if (!runtimeInterfaceType.HasSameTypeDefinition(interfaceType)) | ||||||
| { | ||||||
| // Variance matches require a typedef match | ||||||
| // Equivalence isn't sufficient, and is uninteresting as equivalent interfaces cannot have static virtuals. | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| if (runtimeInterfaceType.CanCastTo(interfaceType)) | ||||||
| { | ||||||
| if (!inVersionBubble(runtimeInterfaceType)) | ||||||
| { | ||||||
| // Fail the resolution if a candidate variant interface match is outside of the version bubble | ||||||
| return null; | ||||||
| } | ||||||
|
|
||||||
| // Attempt to resolve on variance matched interface | ||||||
| MethodDesc runtimeInterfaceMethod = TryResolveInterfaceMethodOnVariantCompatibleInterface(runtimeInterfaceType, interfaceMethod); | ||||||
|
||||||
| MethodDesc runtimeInterfaceMethod = TryResolveInterfaceMethodOnVariantCompatibleInterface(runtimeInterfaceType, interfaceMethod); | |
| MethodDesc runtimeInterfaceMethod = runtimeInterfaceType.FindMethodOnExactTypeWithMatchingTypicalMethod(interfaceMethod); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under what condition could this be null?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'll need to compare with the interfaceMethod.GetMethodDefinition(). The code below assumes interfaceMethod is already instantiated if it's generic but the MethodImpl records you get from the constrained type will not be instantiated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit, thanks for explaining!
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (interfaceMethod.HasInstantiation || methodImpl.Body.HasInstantiation || constrainedType.HasInstantiation) | |
| if (interfaceMethod.HasInstantiation) |
If interfaceMethod.HasInstantiation != methodImpl.Body.HasInstantiation, terrible things will happen below, so we might as well assume that and consider the Body check redundant.
I believe InstantiateSignature is going to be a no-op if the constrained type has an instantiation because that part is already instantiated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit, thanks for the suggestion.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| resolvedMethodImpl = resolvedMethodImpl.InstantiateSignature(constrainedType.Instantiation, interfaceMethod.Instantiation); | |
| resolvedMethodImpl = resolvedMethodImpl.MakeInstantiatedMethod(interfaceMethod.Instantiation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit, thanks for the suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
inVersionBubbleparameter should probably go under#if READYTORUN.We don't ifdef things in the type system, but also we don't put version bubble stuff here. So maybe an ifdef is less evil.
I do wonder why we were able to get by without a bool like this for the other virtual method resolution scenarios.