Skip to content

[cxx-interop] Allow removing elements from std::vector #69433

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
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[cxx-interop] Allow removing elements from std::vector #69433

wants to merge 1 commit into from

Conversation

egorzhdan
Copy link
Contributor

This adds func remove(at index: Int) to all instantiations of std::vector via an extension for protocol CxxVector.

The original C++ method std::vector::erase is not visible in Swift because all of its overloads return unsafe iterators.

rdar://113704853

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

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

The test is failing on Windows because std::vector::iterator is not conformed to the UnsafeCxxRandomAccessIterator protocol:

template <class _Myvec>
class _Vector_iterator : public _Vector_const_iterator<_Myvec> {
public:
    using _Mybase = _Vector_const_iterator<_Myvec>;
// ...
    using _Mybase::operator-; // <-- this is the overload Swift needs

operator- takes _Vector_const_iterator instead of _Vector_iterator as a parameter, preventing the automatic conformance.

@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from c3ffea2 to 3b19e59 Compare November 14, 2023 14:55
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan enabled auto-merge November 14, 2023 15:11
@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 3b19e59 to 523a2c0 Compare November 15, 2023 16:11
This was referenced Nov 20, 2023
Closed
Merged
@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 523a2c0 to 6eb70df Compare November 20, 2023 19:41
@egorzhdan egorzhdan disabled auto-merge November 20, 2023 19:41
@egorzhdan egorzhdan marked this pull request as draft November 20, 2023 19:41
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Linux

@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 6eb70df to 487e8e5 Compare January 2, 2024 19:46
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 487e8e5 to 288a0d6 Compare July 9, 2024 18:29
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 288a0d6 to 88dca49 Compare July 10, 2024 17:26
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

@egorzhdan
[cxx-interop] Allow removing elements from std::vector
fcbfc49 
This adds `func remove(at index: Int)` to all instantiations of `std::vector` via an extension for `protocol CxxVector`.

The original C++ method `std::vector::erase` is not visible in Swift because all of its overloads return unsafe iterators.

rdar://113704853
@egorzhdan egorzhdan force-pushed the egorzhdan/cxx-vector-remove branch from 88dca49 to fcbfc49 Compare September 2, 2024 14:17
@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

2 similar comments
@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

@egorzhdan
Copy link
Contributor Author

@swift-ci please test Windows

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

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

@Xazax-hun Xazax-hun Awaiting requested review from Xazax-hun Xazax-hun will be requested when the pull request is marked ready for review Xazax-hun is a code owner

@j-hui j-hui Awaiting requested review from j-hui j-hui will be requested when the pull request is marked ready for review j-hui is a code owner

@fahadnayyar fahadnayyar Awaiting requested review from fahadnayyar fahadnayyar will be requested when the pull request is marked ready for review fahadnayyar is a code owner

@susmonteiro susmonteiro Awaiting requested review from susmonteiro susmonteiro will be requested when the pull request is marked ready for review susmonteiro is a code owner

@hnrklssn hnrklssn Awaiting requested review from hnrklssn hnrklssn will be requested when the pull request is marked ready for review hnrklssn is a code owner

@beccadax beccadax Awaiting requested review from beccadax beccadax will be requested when the pull request is marked ready for review beccadax is a code owner

@ian-twilightcoder ian-twilightcoder Awaiting requested review from ian-twilightcoder ian-twilightcoder will be requested when the pull request is marked ready for review 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 @hyp