Skip to content

[cxx-interop] Support nested structs #75477

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

Merged
merged 1 commit into from
Sep 10, 2024
Merged

[cxx-interop] Support nested structs #75477

merged 1 commit into from
Sep 10, 2024

Conversation

Xazax-hun
Copy link
Contributor

It is really involved to change how methods and classes are emitted into the header so this patch introduces the impression of nested structs through using statements and still emits the structs themselves as top level structs. It emits them in their own namespace to avoid name collisions. This patch also had to change some names to be fully qualified to avoid some name lookup errors in case of nested structs. Moreover, nesting level of 3 and above requires C++17 because it relies on nested namespaces. Only nested structs are supported, not nested classes.

Since this patch is already started to grow quite big, I decided to put it out for reviews and plan to address some of the shortcomings in a follow-up PR.

rdar://118793469

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Jul 25, 2024
@Xazax-hun Xazax-hun force-pushed the gaborh/nested-structs branch from 16b039e to 120d722 Compare August 6, 2024 10:30
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

@Xazax-hun Xazax-hun force-pushed the gaborh/nested-structs branch from 120d722 to 4e25ca0 Compare September 3, 2024 12:56
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

@Xazax-hun Xazax-hun force-pushed the gaborh/nested-structs branch from 4e25ca0 to 5b3dbf1 Compare September 9, 2024 16:46
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

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

LGTM, but I will admit that I haven't read every single line of code in this patch.

It is really involved to change how methods and classes are emitted into
the header so this patch introduces the impression of nested structs
through using statements and still emits the structs themselves as top
level structs. It emits them in their own namespace to avoid name
collisions. This patch also had to change some names to be fully
qualified to avoid some name lookup errors in case of nested structs.
Moreover, nesting level of 3 and above requires C++17 because it relies
on nested namespaces. Only nested structs are supported, not nested
classes.

Since this patch is already started to grow quite big, I decided to put
it out for reviews and plan to address some of the shortcomings in a
follow-up PR.

rdar://118793469
@Xazax-hun Xazax-hun force-pushed the gaborh/nested-structs branch from 5b3dbf1 to 94b4666 Compare September 10, 2024 12:22
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

@Xazax-hun Xazax-hun merged commit 863d2e4 into main Sep 10, 2024
3 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/nested-structs branch September 10, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants