Skip to content
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

Implement P1425 iterator range constructors for stack and queue #1994

Merged

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Jun 9, 2021

This implements P1425.

While we are at it it also implements LWG-3506 and LWG-3522. (There is nothing to do for LWG-3529 - it's already implemented.)

Addresses #1965
Fixes #1973

@miscco miscco requested a review from a team as a code owner June 9, 2021 16:29
@StephanTLavavej StephanTLavavej added cxx23 C++23 feature LWG Library Working Group issue labels Jun 10, 2021
@StephanTLavavej

This comment has been minimized.

@miscco miscco force-pushed the p1425r4-stack-queue-constructors branch from b3116f8 to ccdf327 Compare June 12, 2021 09:27
@frederick-vs-ja
Copy link
Contributor

Is it intended not to apply LWG-3506 to C++14/17 modes?

@miscco
Copy link
Contributor Author

miscco commented Jun 13, 2021

That is a good point. The maintainers can give direction

@StephanTLavavej StephanTLavavej self-assigned this Jun 16, 2021
tests/std/test.lst Outdated Show resolved Hide resolved
stl/inc/queue Outdated Show resolved Hide resolved
stl/inc/queue Outdated
Comment on lines 275 to 276
template <class _InIt, class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Container& _Cont, const _Alloc& _Al)
Copy link
Member

Choose a reason for hiding this comment

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

According to LWG-3522, all of these priority_queue constructors taking _InIt should be additionally constrained by _Is_iterator, so I think we need to conjunction_v all 4 of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with simple && as I believe we do not really need the short circuiting of conjunction and might avoid some type instantiations

stl/inc/queue Outdated Show resolved Hide resolved
stl/inc/queue Outdated Show resolved Hide resolved
stl/inc/queue Outdated Show resolved Hide resolved
tests/std/tests/P1425R6_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1425R6_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jun 18, 2021
stl/inc/queue Show resolved Hide resolved
@miscco miscco force-pushed the p1425r4-stack-queue-constructors branch from 139a594 to a9e8c3d Compare June 22, 2021 20:01
@StephanTLavavej

This comment has been minimized.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I'll validate and push some changes.

stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/queue Outdated Show resolved Hide resolved
tests/std/tests/P1425R4_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1425R4_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1425R4_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1425R4_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1425R4_queue_stack_constructors/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Aug 13, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 14, 2021
@StephanTLavavej
Copy link
Member

I'm mirroring this to an MSVC-internal PR. It's totally fine to push changes for code review feedback, but please notify me in that case.

using namespace std;

constexpr int some_data[] = {0, 1, 2, 3, 4, 5};
constexpr int additional_data[] = {6, 7, 8, 9, 10, 11};
Copy link
Member

Choose a reason for hiding this comment

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

These variable names are truly inspired. (No change requested.)

@StephanTLavavej StephanTLavavej merged commit 46477a1 into microsoft:main Aug 17, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing this C++23 feature and related LWG issue resolutions! 😻 ✔️ 🚀

@miscco miscco deleted the p1425r4-stack-queue-constructors branch August 21, 2021 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature LWG Library Working Group issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P1425R4 Iterator Pair Constructors For stack And queue
4 participants