Change the ComInterfaceGenerator to output source in one file per interface and use file-scoped types #83055
Conversation
…erator based on the approach we agreed upon.
…interface and use file-scoped types to provide all of the interface information and implementation based on the design we discussed last week.
…e VirtualMethodIndexAttribute generator in the shape tests for that generator specifically.
jkoritzinsky
added
area-System.Runtime.InteropServices
source-generator
ghost
assigned
|
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsThis PR changes our codegen to match the same shape of code as in https://github.com/AaronRobinsonMSFT/ComObjectRedux/pull/7 For each interface with
As part of getting this PR working, the following general improvements to the ComInterfaceGenerator project and the tests were applied:
|
…ic we use to efficiently reassociate methods with their types and they're much simpler anyway as we don't need to even try to do any marshalling.
| } | ||
|
|
||
| context.RegisterConcatenatedSyntaxOutputs(generateNativeInterface, "NativeInterfaces.g.cs"); | ||
| private static string GenerateMarkerInterfaceSource(ComInterfaceContext iface) => $$""" |
There was a problem hiding this comment.
Is this something we'll want to switch to using the SyntaxFactory methods, or should it stay as an interpolated string?
There was a problem hiding this comment.
I think this is a lot cleaner as an interpolated string and making it typed with SyntaxFactory doesn't get us the benefits of syntax nodes that we get in the rest of the project (more structure around wrapping syntax and knowing what type of syntax we're working with at various API boundaries). I'd be curious what @AaronRobinsonMSFT and @elinor-fung think for this case.
| { | ||
| public bool Equals(SyntaxEquivalentNode<T> other) | ||
| { | ||
| return SyntaxEquivalentComparer.Instance.Equals(Node, other.Node); |
There was a problem hiding this comment.
I know the singleton is usually named Instance, but in this case I first thought this would compare the SyntaxNodes by reference/instance equality. I'm fine leaving it, but if you think it might be confusing too, it could be worth renaming.
There was a problem hiding this comment.
We can rename it, but I'd rather wait until after this PR is merged in.
...ystem.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/SyntaxEquivalentNode.cs
Outdated
Show resolved
Hide resolved
jtschuster
left a comment
jtschuster
left a comment
There was a problem hiding this comment.
Looks good, just a couple nits/questions. Thank you!
2 participants
This PR changes our codegen to match the same shape of code as in https://github.com/AaronRobinsonMSFT/ComObjectRedux/pull/7
For each interface with
[GeneratedComInterface], one file will be generated with the following:InterfaceInformationfile-scoped type that defines the implementation ofIIUnknownInterfaceTypefor the interfaceInterfaceImplementationfile-scoped type, defined in the following 3partialdefinitions for simplicity (we can combine these if we so wish):InterfaceInformationtype)IUnknownDerivedAttributeapplied, using the above two file-scoped types.As part of getting this PR working, the following general improvements to the ComInterfaceGenerator project and the tests were applied:
VtableIndexStubGeneratorand generalized to fit the two (now quite different) shapes of the wrapping code.NativeInterfaceUsageportion of our code snippets has now been moved into a specific test in the "shape validation" suite for theVtableIndexStubGeneratoras the validation it does is no longer valid for theComInterfaceGeneratorand providing a separate single test does a better job at validating the intention of the check.