-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Support function pointer types in System.Reflection.Emit
#119935
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
base: main
Are you sure you want to change the base?
Conversation
Tagging subscribers to this area: @dotnet/area-system-reflection-emit |
@dotnet-policy-service agree |
I've now added support for unmanaged calling conventions and custom modifiers, altough there are still some limitations which I outlined in the original issue (#111003). Fully supporting all possible variants of function pointer types will require updates of the reflection API, as far as I'm concerned. Still, my implementation should support everything that C# currently supports, which is why I am opening the PR for review. I understand if this PR cannot be merged until there is comprehensive support for the feature, altough in my opinion it is already 90% of the way there in terms of support for real-life scenarios. |
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.
Pull Request Overview
This PR adds support for function pointer types in System.Reflection.Emit by implementing signature generation for function pointers. The implementation currently supports managed function pointers without custom modifiers (modopt), with unmanaged calling conventions and custom modifiers planned for future work.
Key changes:
- Added function pointer type detection and handling in signature generation
- Implemented calling convention mapping for function pointers
- Added support for custom modifiers on function pointer return types and parameters
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Show resolved
Hide resolved
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Show resolved
Hide resolved
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.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Show resolved
Hide resolved
Could you please add tests as well? |
Done. |
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Outdated
Show resolved
Hide resolved
...raries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs
Show resolved
Hide resolved
The new test is failing:
|
...raries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs
Outdated
Show resolved
Hide resolved
I have pushed commit with a few more test cases for field references. Some of the do not work. Could you please take a look? |
My implementation should be correct now. The C# field references still fail when they contain unmanaged calling conventions or other custom modifiers, but as far as I can tell that's an issue with Here is the relevant code section (starting at line 737 in case FieldInfo field:
Type declaringType = field.DeclaringType!;
if (declaringType.IsGenericTypeDefinition)
{
//The type of the field has to be fully instantiated type.
declaringType = declaringType.MakeGenericType(declaringType.GetGenericArguments());
}
Type fieldType = ((FieldInfo)GetOriginalMemberIfConstructedType(field)).FieldType; // <- This loses all calling conventions and modifiers
memberHandle = AddMemberReference(field.Name, GetTypeHandle(declaringType),
MetadataSignatureHelper.GetFieldSignature(fieldType, field.GetRequiredCustomModifiers(), field.GetOptionalCustomModifiers(), this));
break; Using This is also an important issue that should be fixed, but it goes beyond the scope of this PR, in my opinion. |
src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jan Kotas <[email protected]>
...raries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs
Outdated
Show resolved
Hide resolved
...raries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs
Show resolved
Hide resolved
Sounds good. We can use the existing issue to track this or create a new one - either way is fine with me. |
Co-authored-by: Jan Kotas <[email protected]>
Co-authored-by: Jan Kotas <[email protected]>
In my opinion it should be a separate issue. I don't know if this is at all feasible, but I think it would be great if function pointers were treated specially in the sense that they would always preserve calling conventions and modifiers in their |
il.Emit(OpCodes.Ldsfld, typeof(ClassWithFunctionPointerFields).GetField("field1")); | ||
il.Emit(OpCodes.Pop); | ||
// References to fields with unmanaged calling convention are broken | ||
// [ActiveIssue("https://github.com/dotnet/runtime/issues/111003")] |
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 you would like this to be tracked by a new issue, could you please create one and point this comment to it instead of the existing one?
This fixes #111003. The current implementation is a work in progress, only managed function pointers with no modopts are supported.
I imagine adding support for unmanaged calling conventions and custom modifiers will be a bit more challenging, since I cannot use
Type.GetFunctionPointerCallingConventions
et al.