Skip to content

[cxx-interop] Check the presence of copy constructor correctly #73297

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 1 commit into from
Oct 29, 2024
Merged

[cxx-interop] Check the presence of copy constructor correctly #73297

merged 1 commit into from
Oct 29, 2024

Conversation

egorzhdan
Copy link
Contributor

@egorzhdan egorzhdan commented Apr 26, 2024 •
edited
Loading

This removes a longstanding workaround in the import logic for C++ structs:
Swift assumed that if a C++ struct has no copy constructor that is explicitly deleted, then the struct is copyable. This is not actually correct. This replaces the workaround with a proper check for the presence of a C++ copy constructor.

rdar://136838485

@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Apr 26, 2024
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test source compatibility

@egorzhdan
Copy link
Contributor Author

@swift-ci please test macOS

2 similar comments
@egorzhdan
Copy link
Contributor Author

@swift-ci please test macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please test macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from c490814 to aaa9b18 Compare July 22, 2024 19:02
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test source compatibility

@egorzhdan egorzhdan requested a review from Xazax-hun July 22, 2024 19:03
@egorzhdan egorzhdan marked this pull request as ready for review July 22, 2024 19:03
@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain macOS

@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from aaa9b18 to bacfc4f Compare September 27, 2024 17:23
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain macOS

@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from bacfc4f to d67c2e5 Compare October 18, 2024 15:01
@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

5 similar comments
@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

4 similar comments
@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

@egorzhdan egorzhdan removed request for xedin and hborla October 25, 2024 23:39
@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from 4fef9b1 to 3f160a6 Compare October 25, 2024 23:51
@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from 5cdf941 to 3f160a6 Compare October 26, 2024 23:17
@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from 3f160a6 to 1c5eacd Compare October 28, 2024 12:57
@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Linux

@egorzhdan
[cxx-interop] Check the presence of copy constructor correctly
fbbec48 
This removes a longstanding workaround in the import logic for C++ structs:
Swift assumed that if a C++ struct has no copy constructor that is explicitly deleted, then the struct is copyable. This is not actually correct. This replaces the workaround with a proper check for the presence of a C++ copy constructor.

rdar://136838485
@egorzhdan egorzhdan force-pushed the egorzhdan/correct-check-for-copy-ctor branch from 1c5eacd to fbbec48 Compare October 28, 2024 20:06
@egorzhdan
Copy link
Contributor Author

@swift-ci please test

@egorzhdan
Copy link
Contributor Author

@swift-ci please build toolchain Ubuntu 22.04

Xazax-hun
Xazax-hun approved these changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@Xazax-hun Xazax-hun left a comment

Choose a reason for hiding this comment

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

LGTM!

@egorzhdan egorzhdan merged commit ffb22d7 into main Oct 29, 2024
6 checks passed
@egorzhdan egorzhdan deleted the egorzhdan/correct-check-for-copy-ctor branch October 29, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xazax-hun Xazax-hun Xazax-hun approved these changes

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

@hyp hyp Awaiting requested review from hyp

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

@ian-twilightcoder ian-twilightcoder Awaiting requested review from ian-twilightcoder ian-twilightcoder is a code owner

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
@egorzhdan @Xazax-hun