Skip to content

Use iterator concept in vector's range constructor - #1794

Merged
Stephan T. Lavavej (StephanTLavavej) merged 14 commits into
microsoft:mainfrom
AdamBucior:vector-range-constructor
Jun 12, 2022
Merged

Use iterator concept in vector's range constructor#1794
Stephan T. Lavavej (StephanTLavavej) merged 14 commits into
microsoft:mainfrom
AdamBucior:vector-range-constructor

Conversation

@AdamBucior

Copy link
Copy Markdown
Contributor

Fixes #1709.

@AdamBucior
Adam Bucior (AdamBucior) requested a review from a team as a code owner March 31, 2021 17:55
@miscco

Copy link
Copy Markdown
Contributor

Welcome to container hell, please take a seat

@miscco

Copy link
Copy Markdown
Contributor

You might also have a look at _Assign_range

@AdamBucior

Copy link
Copy Markdown
Contributor Author

Could someone explain to me why move_iterator's iterator_concept is input_iterator_tag?

Comment thread stl/inc/vector Outdated
Comment thread stl/inc/vector

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After merging #1771, there are non-trivial merge conflicts in <vector>.

@AlexGuteniev

Copy link
Copy Markdown
Contributor

Also counts towards #189

@StephanTLavavej

Copy link
Copy Markdown
Member

Could someone explain to me why move_iterator's iterator_concept is input_iterator_tag?

I believe this is because a Forward Iterator can be read multiple times (the idea is that they're like forward_list - they support only forward traversal, but otherwise point to persistent objects in memory). Input Iterators can be read only once (like an istream_iterator, you can't store multiple "locations" to read previous values). move_iterator is weird because although it can wrap something with very strong traversal (up to and including random access), the automatic-moving breaks multi-read behavior. Thus, describing move_iterator with input_iterator_tag is the strictly correct thing to do.

Apologies for the year-long delay in answering this question 😹

@AdamBucior

Copy link
Copy Markdown
Contributor Author

Stephan T. Lavavej (@StephanTLavavej) It seems that it might change with P2520.

@StephanTLavavej

This comment was marked as resolved.

Comment thread stl/inc/vector Outdated
Comment thread tests/std/tests/VSO_0000000_fancy_pointers/test.compile.pass.cpp
Comment thread stl/inc/vector
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks, this looks great! I found only minor issues while video reviewing, so I went ahead and validated and pushed changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an interesting extension, since it's almost exactly the change WG21-P2408 will have us making to the non-permuting std algorithms for C++23. I think we're going to be seeing a lot of similar relaxations for "old" standard library code in C++26 as WG21 is getting our brains wrapped around where and how we can make these integrations without breaking programs in the wild.

@StephanTLavavej

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit 877caba into microsoft:main Jun 12, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

Thanks for improving the performance of the STL's most important data structure! 😻 🚀 🚀

@AdamBucior
Adam Bucior (AdamBucior) deleted the vector-range-constructor branch June 12, 2022 10:27
Igor Zhukov (fsb4000) pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
@drsnuggles8

drsnuggles8 commented Oct 20, 2022

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Must go faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance issue in handling range iterators in vector constructor

6 participants