Skip to content

[interop][SwiftToCxx] Add initial support for passing/returning enums #59584

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 23, 2022
Merged

[interop][SwiftToCxx] Add initial support for passing/returning enums #59584

merged 4 commits into from
Jun 23, 2022

Conversation

WANGJIEKE
Copy link
Contributor

@WANGJIEKE WANGJIEKE commented Jun 20, 2022 •
edited
Loading

Add initial support for passing/returning Swift enums between Swift and C++:

  • Unify logic for Swift structs and enums since they behave similarly
  • Modify existing enum-related functions to support C++ interop mode
  • Create necessary test cases

@hyp Could you review this pull request? I also got some questions and I believe they're shown below.

WANGJIEKE
WANGJIEKE commented Jun 20, 2022
View reviewed changes
WANGJIEKE
WANGJIEKE commented Jun 20, 2022
View reviewed changes
@hyp hyp added the c++ interop Feature: Interoperability with C++ label Jun 20, 2022
@hyp
Copy link
Contributor

hyp commented Jun 20, 2022

@swift-ci please test

@WANGJIEKE WANGJIEKE marked this pull request as ready for review June 21, 2022 00:13
@hyp
Copy link
Contributor

hyp commented Jun 21, 2022

I will finish reviewing in the morning , but so far implementation looks good

@hyp
Copy link
Contributor

hyp commented Jun 21, 2022

@swift-ci please test source compatibility

@hyp
Copy link
Contributor

hyp commented Jun 21, 2022

This looks good, but I have one request for the tests.

Could you expand on the test for small and tiny enums to also include CHECK lines for the stub structs and pass/return functions?

swift_interop_passDirect_Enums_Small, swift_interop_returnDirect_Enums_Tiny, stub_Enums_Tiny etc.

@WANGJIEKE
Copy link
Contributor Author

Sure.

@hyp
Copy link
Contributor

hyp commented Jun 22, 2022

@swift-ci please test

@hyp
Copy link
Contributor

hyp commented Jun 22, 2022

@swift-ci please test source compatibility

@hyp
Copy link
Contributor

hyp commented Jun 22, 2022

@swift-ci please test source compatibility

@hyp
Copy link
Contributor

hyp commented Jun 22, 2022

@swift-ci please test

@hyp
Copy link
Contributor

hyp commented Jun 22, 2022

@WANGJIEKE one more request, could you add // UNSUPPORTED: CPU=arm64e to your execution tests in this patch? We have some trouble with value witness table on arm64e in CI so for now we have to disable it for arm64e.

@WANGJIEKE
Copy link
Contributor Author

Added

@hyp
Copy link
Contributor

hyp commented Jun 23, 2022

@swift-ci please test

@hyp
Copy link
Contributor

hyp commented Jun 23, 2022

@swift-ci please test source compatibility

@hyp hyp merged commit c6ff96f into swiftlang:main Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyp hyp hyp approved these changes

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WANGJIEKE @hyp