-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Derived Interface support in ComInterfaceGenerator #84271
Conversation
Implement support for deriving COM interfaces to append their members to the vtables of their base interface type.
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsImplement support for deriving COM interfaces to append their members to the vtables of their base interface type. This PR also includes a design doc about the derived-interface feature set. This PR implements the first part of the design (it does not implement the "Designing for Performance" section of the spec, as implementing this section is not a breaking change (whereas the first part of the design is).
|
…terface to ensure we don't break later steps.
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/gen/ComInterfaceGenerator/ComInterfaceGenerator.cs
Outdated
Show resolved
Hide resolved
@@ -66,32 +68,84 @@ public void Initialize(IncrementalGeneratorInitializationContext context) | |||
var interfacesToGenerate = interfacesWithDiagnostics.Where(static data => data.Diagnostic is null); | |||
var invalidTypeDiagnostics = interfacesWithDiagnostics.Where(static data => data.Diagnostic is not null); | |||
|
|||
var interfaceContexts = interfacesToGenerate.Select((data, ct) => | |||
var interfaceBaseInfo = interfacesToGenerate.Collect().SelectMany((data, ct) => |
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.
Just a thought, for one pass it probably is better to Collect this and be able to cache the counts, but during incremental passes, this will make us recalculate offsets for all interfaces when 1 is changed, right? Might be premature optimization to change it to something else now, but we could make a note about it for the future.
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'd rather do the cache right away as doing this without a cache becomes an O(n^2)
algorithm almost immediately and heavily derived interface types aren't uncommon (DirectX has cases where they're up to ID3D12Device9 and we have some examples of in dotnet/runtime that go pretty deep like ISOSDacInterface, which is up to ISOSDacInterface13).
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.
Looks great, thank you!
|
||
In the ComInterfaceGenerator, we want to improve the experience when writing COM interfaces that derive from other COM interfaces. The built-in system has some quirks due to differences between how interfaces work in C# in comparison to COM interfaces in C++. | ||
|
||
## COM Interface Inheritance in C++ |
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.
Thoughts on adding the sections about how it works in C++ and .NET ComImport (and then GeneratedComInterface once it ships) to public docs? Probably somewhere under https://learn.microsoft.com/dotnet/standard/native-interop/cominterop
I think the descriptions you have here are clear and useful. I don't think we have this called out or explained anywhere in official docs.
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.
That's a good idea! I'll make sure to add it to the docs.
Test failures are all known. |
Implement support for deriving COM interfaces to append their members to the vtables of their base interface type.
This PR also includes a design doc about the derived-interface feature set. This PR implements the first part of the design (it does not implement the "Designing for Performance" section of the spec, as implementing this section is not a breaking change (whereas the first part of the design is).