-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[cxx-interop] Add a basic test for using std::iterator_traits
#41290
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
[cxx-interop] Add a basic test for using std::iterator_traits
#41290
Conversation
d91b9c8
to
594f464
Compare
@hyp |
std::iterator_traits
bdb11fd
to
265b8f2
Compare
@swift-ci please test |
Build failed |
265b8f2
to
3945f47
Compare
@swift-ci please test |
@zoecarver when you get a chance, could you merge this for me? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I don't think that this really makes sense to test. If we are able to construct an object, it won't be nil. It might be interesting to see if we can import iterator_traits
(a module interface test) but honestly, I'm not even sure how valuable that would be. I don't think people will be using traits like this very often in Swift.
What would be really helpful is some tests for std::next
and std::prev
(if you have time, std::distance
, and std::advance
might be interesting too) because I anticipate those will be used very frequently. Could you update this PR to test those instead (or make a new PR)?
I really appreciate you spending time writing tests! So, I'm sorry if this is disappointing :(
I should also mention that these tests look 100% correct, so really well done for your first PR! It's just the thing you're testing doesn't really make sense. |
Thank you! All good, excited that i can contribute and get feedback, it's the best way to learn. I completely follow and I will spend some time adding tests for Thank you for taking the time to go over this stuff with me! I really appreciate it |
Add basic tests for using
std::iterator_traits
Ref: #41166