-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fix NativeMarshalling attribute override for COM interfaces #121643
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
Modified ComInterfaceMarshallingInfoProvider to check for NativeMarshallingAttribute on COM interfaces and defer to that parser when present. Added tests to verify the fix works for both ComInterfaceGenerator and LibraryImportGenerator. Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
|
@copilot you need to add behavioral tests to ComInterfaceGenerator.Tests.csproj. Add a test that gets the same ComInterface instance from a LibraryImportMethod twice and validate that they are not the same instance. Also add a method on the interface that returns a COM interface. The implementation should return 'this'. In the test, validate that the returned instance is a new object, not the same cached instance. |
Added tests to ComInterfaceGenerator.Tests that verify: 1. Getting the same COM interface twice returns unique instances when using NativeMarshallingAttribute with UniqueComInterfaceMarshaller 2. COM interface methods returning the same interface type return new managed instances, not cached ones Created IUniqueMarshalling interface with NativeMarshallingAttribute, native exports, and comprehensive tests. Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Added behavioral tests in commit 3d67609. The tests verify:
Created IUniqueMarshalling interface with |
|
@copilot You should be able to use StrategyBasedComWrappers in UniqueMarshalling.CreateComObject without having to create a new ComWrappers implementation. Delete MyComWrapper and use StrategyBasedComWrappers instead. Create a SharedTypes.ComInterfaces.UniqueMarshalling instance instead of ImplementingObject. |
Removed custom MyComWrapper implementation and ImplementingObject class. Now using StrategyBasedComWrappers directly with SharedTypes.ComInterfaces.UniqueMarshalling instance, which leverages the GeneratedComClass attribute to handle the COM vtable automatically. Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Simplified in commit 1a37329. Removed MyComWrapper and ImplementingObject classes. Now using StrategyBasedComWrappers directly with SharedTypes.ComInterfaces.UniqueMarshalling instance, which uses the GeneratedComClass attribute to automatically handle the COM vtable. |
…parser order, not arbitrary attribute order.
...me.InteropServices/tests/TestAssets/NativeExports/ComInterfaceGenerator/UniqueMarshalling.cs
Show resolved
Hide resolved
Updated comment to clarify that calling NewUniqueMarshalling() twice returns different managed instances for the same COM object, addressing code review feedback. Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
Added IUniqueMarshalling.cs as a Compile item in LibraryImportGenerator.Tests.csproj to enable the generator to process the COM interface definition. Moved the GetSameComInterfaceTwiceReturnsUniqueInstances test to LibraryImportGenerator.Tests since it tests LibraryImport marshalling behavior. Kept MethodReturningComInterfaceReturnsUniqueInstance in ComInterfaceGenerator.Tests as it tests COM interface method behavior. Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/interop-contrib |
|
@AaronRobinsonMSFT PTAL when you have a chance |
...stem.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/MarshallingInfoParser.cs
Outdated
Show resolved
Hide resolved
…erop.SourceGeneration/MarshallingInfoParser.cs Co-authored-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Fix NativeMarshalling attribute override for COM interfaces
Plan
Changes Made
MarshallingInfoParser.cs & ComInterfaceMarshallingInfoProvider.cs: Improved implementation by the maintainer to iterate through parsers in order (NativeMarshallingAttributeParser before ComInterfaceMarshallingInfoProvider) rather than iterating through attributes, ensuring NativeMarshallingAttribute is always checked first.
Unit Tests: Added compilation tests to verify the generators handle NativeMarshalling correctly
Behavioral Tests:
[NativeMarshalling(typeof(UniqueComInterfaceMarshaller<IUniqueMarshalling>))]Test Results
Issue Summary
When a COM interface has both
[GeneratedComInterface]and[NativeMarshalling(typeof(UniqueComInterfaceMarshaller<IFoo>))], the generator should use the custom marshaller specified inNativeMarshallingAttribute. Previously, it always used the defaultComInterfaceMarshaller<T>.The fix ensures that when
NativeMarshallingAttributeis present on a COM interface, it takes precedence over the default marshaller by ensuring the parser order checks NativeMarshallingAttributeParser before ComInterfaceMarshallingInfoProvider.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.