Skip to content

Conversation

@jevansaks
Copy link
Member

@jevansaks jevansaks commented Sep 5, 2025

We want to use CsWin32 to generate WinRT COM interfaces from header files that are not in the Windows SDK, but they derive from IInspectable. CsWin32 was not correctly handling these, especially in the mode of allowMarshalling=false.

The problem was that we weren't being consistent about which MetadataReader we were using to resolve typedefs, methoddefs, and other things out of the winmd files when we crossed the winmd boundaries. In this PR we try to more clearly formalize how we access these things. Now we make sure to that the Generator we pass a TypeDefinition to (or whatever) has the owning MetadataReader for it.

The biggest source of problems for this was HandleTypeHandleInfo, so I changed this to get its Generator when it's created. This meant I needed to pass the Generator down to the SignatureHandleProvider and make it bound to the Generator instead of a singleton. But now HandleTypeHandleInfo always has the Generator that it can use instead of needing the one from the TypeSyntaxSettings.

This PR adds a test that covers this scenario and we run through marshalling/non-marshalling and net472 and net8 combinations.

@jevansaks jevansaks changed the title [DRAFT] Support for derived IInspectable COM interfaces in custom winmds Support for derived IInspectable COM interfaces in custom winmds Sep 6, 2025
@jevansaks jevansaks marked this pull request as ready for review September 6, 2025 03:34
Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few touch-ups suggested, but the only real blocking issue is the mixing up of MetadataFile objects between generators, as my comments explain.

Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Much better. Almost there.
I'd fix it up myself but then I wouldn't be able to approve the PR 😒

@AArnott
Copy link
Member

AArnott commented Sep 10, 2025

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@AArnott AArnott merged commit c676a8c into microsoft:main Sep 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants