-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Implement unmanaged-to-managed direction for vtable stub generator #77130
Implement unmanaged-to-managed direction for vtable stub generator #77130
Conversation
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsImplement the unmanaged-to-managed direction for stubs generated with the VTableIndexStubGenerator. This PR also includes a design doc for how to handle exception handling at the unmanaged-to-managed boundary, which this PR will also implement. TODO:
|
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.
Can you share an example of what the generated source looks like?
...opServices/gen/ComInterfaceGenerator/Marshallers/NativeThisToManagedThisMarshallerFactory.cs
Outdated
Show resolved
Hide resolved
...ime.InteropServices/gen/Microsoft.Interop.SourceGeneration/NativeToManagedStubCodeContext.cs
Outdated
Show resolved
Hide resolved
...ime.InteropServices/gen/Microsoft.Interop.SourceGeneration/NativeToManagedStubCodeContext.cs
Outdated
Show resolved
Hide resolved
Once I've done more work and gotten this PR to at least some semblance of green, I'll add some example generated code to the top of this PR. |
…rect types and the unmanaged/native names for the parameters.
…other (commented out for now) case that we need to fix.
… and not a TypeSyntax. We need to reason about this type for the unmanaged exception handling design, so using a ManagedTypeInfo is preferable.
…ether we're marshalling a parameter/return value/etc from managed to unmanaged or vice versa. This abstraction will be useful when enabling unmanaged->managed stubs as we won't need to go update every marshalling generator to correctly understand what to do. Also rename some members from "in/out/ref" to use the direction-based names.
…ion unit tests passing.
I've pushed all of the changes to this PR, but I'm going to spin off some of these commits into separate PRs to make them more reviewable. I'll mark this as ready for review once those PRs are merged. |
…he user-allocated vtable.
...em.Runtime.InteropServices/gen/ComInterfaceGenerator/ManagedToNativeVTableMethodGenerator.cs
Outdated
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Show resolved
Hide resolved
...stem.Runtime.InteropServices/tests/Ancillary.Interop/IUnmanagedVirtualMethodTableProvider.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.
Please give @elinor-fung a chance to review this, but I am good.
docs/design/libraries/ComInterfaceGenerator/UnmanagedToManagedEH.md
Outdated
Show resolved
Hide resolved
...es/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypePositionInfo.cs
Outdated
Show resolved
Hide resolved
...es/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypePositionInfo.cs
Show resolved
Hide resolved
All test failures are unrelated. @elinor-fung any more feedback before I merge this? |
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 it would still be helpful to put examples of the generated source in the PR description. Beyond the PR itself, I have found it helpful when looking through previous changes when reworking (or breaking) things when we were working on LibraryImport.
I think we also need tests that would have caught the exception marshalling wiring (fine with that in a separate change) .
...ies/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/BoundGenerators.cs
Outdated
Show resolved
Hide resolved
...es/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypePositionInfo.cs
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Outdated
Show resolved
Hide resolved
...nteropServices/gen/ComInterfaceGenerator/Marshallers/NativeToManagedThisMarshallerFactory.cs
Outdated
Show resolved
Hide resolved
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Outdated
Show resolved
Hide resolved
I'll add the runtime tests for ExceptionMarshalling in a separate change. |
...braries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/VtableIndexStubGenerator.cs
Outdated
Show resolved
Hide resolved
…nMarshalling explicitly to Custom to match the design for StringMarshalling
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.
Thanks for adding those generated code examples!
Failure is unrelated. |
Implement the unmanaged-to-managed direction for stubs generated with the VTableIndexStubGenerator.
This PR also includes a design doc for how to handle exception handling at the unmanaged-to-managed boundary, which this PR implements.
Generated Code (NativeToManagedStubs.g.cs)
Generated code (PopulateVTable.g.cs)