Adjust managed type system for new function pointer handling #84819
Conversation
After dotnet#81006, the calling convention is no longer part of the type system identity of a function pointer type - it serves more like a modopt as far as the type system is concerned. The type system only cares whether the pointer is managed or not. Adjust the managed type system accordingly: * If we're reading/representing a standalone method signature, read it as usual. Calling convention is available in flags/modopts. * If we're reading/representing a function pointer type, collapse the calling convention information into the managed/unmanaged bit only.
ghost
assigned 
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsAfter #81006, the calling convention is no longer part of the type system identity of a function pointer type - it serves more like a modopt as far as the type system is concerned. The type system only cares whether the pointer is managed or not. Adjust the managed type system accordingly:
Cc @dotnet/ilc-contrib @dotnet/crossgen-contrib
|
| @@ -371,7 +371,19 @@ private MethodSignature ParseMethodSignatureImpl(bool skipEmbeddedSignatureData) | |||
| Debug.Assert((int)MethodSignatureFlags.CallingConventionVarargs == (int)SignatureCallingConvention.VarArgs); | |||
| Debug.Assert((int)MethodSignatureFlags.UnmanagedCallingConvention == (int)SignatureCallingConvention.Unmanaged); | |||
|
|
|||
| flags = (MethodSignatureFlags)signatureCallConv; | |||
| // If skipEmbeddedSignatureData is true, we're building the signature for the purposes of building a type. | |||
| // We normalize unmanaged calling convention into a single value - "unmanaged". | |||
There was a problem hiding this comment.
Not sure I understand why this is safe. You mean that, after the other change, it no longer matters if you have
delegate* unmanaged<int> p
vs.
delegate* unmanaged<[SomeModReq]int> p
We effectively erase custom modifiers for unmanaged function pointers?
There was a problem hiding this comment.
Modifiers were already erased for runtime pointer types - that's what the existing skipEmbeddedSignatureData argument controls. It distinguishes between "are we parsing a standalone method signature" vs "are we parsing a TypeSpec that is a function pointer typespec". Custom modifiers never matter for type identity - modifiers only matter for signature resolution, but they don't contribute to identity.
We have some tests testing this like:
runtime/src/tests/reflection/Modifiers/modifiersdata.il
Lines 160 to 166 in ed0f1c5
The logic I'm adding basically switches specific calling conventions into things that act like modifiers (previously they would be part of the type identity).
The added unit test is then testing two things:
- That we still consider signatures that differ by modopts to be different signatures
- The fact that asking for the type of the method parameter only distinguishes managed/unmanaged
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@sbomer could the extra-platforms NativeAOT linker failures be related to the build system switchover? (On my phone now and it's going to be weekend so not investigating) |
|
Almost certainly caused by #84148. Taking a look. |
| @@ -27,7 +27,8 @@ public enum EmbeddedSignatureDataKind | |||
| { | |||
| RequiredCustomModifier = 0, | |||
| OptionalCustomModifier = 1, | |||
| ArrayShape = 2 | |||
| ArrayShape = 2, | |||
| UnmanagedCallConv = 3, | |||
There was a problem hiding this comment.
Why do we need this as a EmbeddedSignatureDataKind instead of as a MethodSignatureFlags?
I think I might be missing something. It seems like this change applies to the nested arguments of a function pointer, rather than the function pointer itself. Is that right? Does that mean that this is only for function pointers of function pointers?
There was a problem hiding this comment.
EmbeddedSignatureData is attached to method signatures, but we can decide which method signature it should attach to.
Consider: void Blah(delegate*unmanaged[Cdecl]<void> x);. When we're materializing the topmost parent method signature, we set a flag in the parser that it should generate embedded signature data. Embedded signature data is a sidecar that gets attached to the topmost method signature. It can express things in arbitrary type construction nesting. It's really weird. Back to the signature - we want to attach the modopt (and non-meaningful flags) to the topmost EmbeddedSignatureData. If we were to attach it to the nested signature, it would become part of the identity. But we don't want it part of the identity of the delegate*unmanaged[Cdecl]<void> type because it was decided that for the type system purposes, the only bit that matters is whether the method is unmanaged (and not the exact calling convention).
There was a problem hiding this comment.
Ah it was avoiding putting it in the identity that I missed
That's a good point. We should have updated the ECMA-335 amendments doc in #81006 because that PR changed things. It does seem to affect I.8.7 Assignment compatibility at minimum: "iii) Any modopt, modreq, or pinned modifiers are ignored; and I'll make a separate PR for this. |
After #81006, the calling convention is no longer part of the type system identity of a function pointer type - it serves more like a modopt as far as the type system is concerned. The type system only cares whether the pointer is managed or not. Adjust the managed type system accordingly:
Cc @dotnet/ilc-contrib @dotnet/crossgen-contrib