Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

User/benkuhn/struct ordering#1052

Merged
kennykerr merged 4 commits intomastermicrosoft/cppwinrt:masterfrom
user/benkuhn/struct-orderingmicrosoft/cppwinrt:user/benkuhn/struct-orderingCopy head branch name to clipboard
Oct 28, 2021
Merged

User/benkuhn/struct ordering#1052
kennykerr merged 4 commits intomastermicrosoft/cppwinrt:masterfrom
user/benkuhn/struct-orderingmicrosoft/cppwinrt:user/benkuhn/struct-orderingCopy head branch name to clipboard

Conversation

@BenJKuhn
Copy link
Member

@BenJKuhn BenJKuhn commented Oct 28, 2021

Fixes #1049. Partly.

cppwinrt separates the headers by namespace, and within each namespace into 'passes' where each pass has specific responsibilities. Structs for a given namespace all appear in the same header. There are limits to what C++ /WinRT can resolve in terms of struct dependencies across headers, but within a header, it sorts definitions so that structs consumed by other structs appear first. In the absence of dependencies, it follows metadata order, which is alphabetical.

There's an overlooked corner case in the dependency sorting. structs generally only contain plain data, with two exceptions: strings and nullable types (instances of IReference). The dependency checking code only looked at the exact type name when evaluating dependencies. If a struct contains an IReference<> to another struct, the dependency checking code would not detect the template parameter inside the IReference type.

Since this is a very constrained special case, I've chosen to code a fix that looks specifically for IReference. MidlRT and other tooling enforce this constraint. It could be generalized to parse template parameters out of the type name, but given there's only one exact type that can appear, this is complete and sufficient as written.

@combinatorial
Copy link

Did you intend to tag issue #1049 ? #1047 seems unrelated to my (non-expert) eyes.

@kennykerr kennykerr merged commit bddd2e1 into master Oct 28, 2021
@kennykerr kennykerr deleted the user/benkuhn/struct-ordering branch October 28, 2021 22:42
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.

Structures that use IReference generated in alphabetical order instead of topological order

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.