- 
                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
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
              
                Outdated
          
            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
              
                Outdated
          
            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
              
                Outdated
          
            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  | 
        
          
                ...raries/System.Reflection.Emit/tests/PersistedAssemblyBuilder/AssemblySaveTypeBuilderTests.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                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
              
                Outdated
          
            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
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                src/libraries/System.Reflection.Emit/src/System/Reflection/Emit/SignatureHelper.cs
          
            Show resolved
            Hide resolved
        
      | /ba-g timeouts - The job running on agent ... ran longer than the maximum time of 150 minutes | 
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.
Thank you!
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.GetFunctionPointerCallingConventionset al.