Skip to content

[cxx-interop] windows methods fixes #67268

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 2 commits into from
Jul 21, 2023
Merged

[cxx-interop] windows methods fixes #67268

merged 2 commits into from
Jul 21, 2023

Conversation

hyp
Copy link
Contributor

@hyp hyp commented Jul 12, 2023

No description provided.

@hyp hyp added the c++ interop Feature: Interoperability with C++ label Jul 12, 2023
@hyp hyp requested review from zoecarver and egorzhdan as code owners July 12, 2023 20:40
@hyp
Copy link
Contributor Author

hyp commented Jul 12, 2023

@swift-ci please test

@hyp hyp marked this pull request as draft July 12, 2023 20:42
@hyp
Copy link
Contributor Author

hyp commented Jul 12, 2023

@swift-ci please test windows platform

10 similar comments
@hyp
Copy link
Contributor Author

hyp commented Jul 13, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 13, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 13, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 13, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 13, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test windows platform

@hyp
[cxx-interop] Fix the windows ABI for returning indirect values out o…
c09135b 
…f methods

Fixes swiftlang#66326

This allows us to reneable Windows method tests. Note that Windows still has
a broken convention for non-trivial record with non-trivial destructor but
trivial copy-constructor, so classes in the methods.swift test need an explicit
copy constructor.

Fixes rdar://88391102
@hyp hyp force-pushed the eng/windows-method-oh-no branch from b6fff32 to c09135b Compare July 14, 2023 22:56
@hyp
Copy link
Contributor Author

hyp commented Jul 14, 2023

@swift-ci please test

@hyp hyp changed the title [cxx-interop] start investigating windows method misfortunes [cxx-interop] windows methods fixes Jul 14, 2023
@hyp
Copy link
Contributor Author

hyp commented Jul 17, 2023

@swift-ci please test windows platform

9 similar comments
@hyp
Copy link
Contributor Author

hyp commented Jul 17, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 18, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 19, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 19, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 19, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 19, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 19, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test windows platform

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test linux platform

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test windows platform

@hyp
[cxx-interop] Windows: unify address-only logic and mark non-trivial …
96806f4 
…loadable C++ types as unavailable

Windows logic for determining address-only type layout for a C++ type is now unified with other platforms.
However, this means that on Windows, a C++ type with a custom destructor, but a default copy constructor
is now loadable, even though it's non-trivial. Since Swift does not support such type operations at the
moment (it can't be yet destroyed), mark such type as unavailable in Swift instead, when building for
the Windows target.

This fixes the Windows miscompilation related to such types when they were passed indirectly to C++
functions even though they're actually passed directly.
@hyp hyp force-pushed the eng/windows-method-oh-no branch from 34bd560 to 96806f4 Compare July 20, 2023 22:03
@hyp hyp marked this pull request as ready for review July 20, 2023 22:03
@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test

@hyp
Copy link
Contributor Author

hyp commented Jul 20, 2023

@swift-ci please test source compatibility

This was referenced Jul 20, 2023
Merged
Closed
This was linked to issues Jul 20, 2023
[cxx-interop] fix the C++ methods dispatch for the Windows target #66326
Closed
[SR-13129] [IRGen, Windows] Wrong argument order when calling C++ member functions with indirect return types #55575
Closed
@hyp hyp merged commit 15a0345 into swiftlang:main Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zoecarver zoecarver Awaiting requested review from zoecarver zoecarver is a code owner

@egorzhdan egorzhdan Awaiting requested review from egorzhdan egorzhdan is a code owner

Assignees
No one assigned
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Milestone
No milestone
1 participant
@hyp