Skip to content

C++ Interop: tweak subscript operator tests #38351

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
Jul 13, 2021

Conversation

egorzhdan
Copy link
Contributor

C++ structs that define a non-default copy constructor should actually copy their members in the copy constructor:
Swift might copy the struct around, and if it does so after the setValueAtIndex call, the modification of NonTrivialIntArrayByVal::values won't propagate to the copied struct and these tests no longer pass:
Interop/Cxx/operators/member-inline.swift
Interop/Cxx/operators/member-out-of-line.swift

The tests are currently passing on the main branch despite this, but they might fail after a seemingly unrelated change (which is how I discovered this).

C++ structs that define a non-default copy constructor should actually copy their members in the copy constructor:
Swift might copy the struct around, and if it does so after the `setValueAtIndex` call, the modification of `NonTrivialIntArrayByVal::values` won't propagate to the copied struct and these tests no longer pass:
• `Interop/Cxx/operators/member-inline.swift`
• `Interop/Cxx/operators/member-out-of-line.swift`

The tests are currently passing on the main branch despite this, but they might fail after a seemingly unrelated change (which is how I discovered this).
@egorzhdan egorzhdan added the c++ interop Feature: Interoperability with C++ label Jul 12, 2021
@egorzhdan
Copy link
Contributor Author

cc @plotfi @zoecarver

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan requested a review from zoecarver July 12, 2021 19:32
Copy link
Contributor

@zoecarver zoecarver left a comment

Choose a reason for hiding this comment

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

This is an obviously correct NFC. Thanks. 🛳

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test macOS

@egorzhdan egorzhdan merged commit 16b6306 into swiftlang:main Jul 13, 2021
@egorzhdan egorzhdan deleted the cxx-tests-copy-ctor branch July 13, 2021 19:22
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