This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Implement the resolutions for LWG3506 and LWG3522.
ClosedPublic

Authored by Quuxplusone on Jul 26 2021, 2:09 PM.

Details

Reviewers
ldionne
tcanens
Group Reviewers
Restricted Project
Summary
Implement the changes in all language modes.

LWG3506 "Missing allocator-extended constructors for priority_queue"
makes the following changes:
- New allocator-extended constructors for priority_queue.
- New deduction guides targeting those constructors.

LWG3522: "Missing requirement on InputIterator template parameter
for priority_queue constructors". The iterator parameter should be
constrained to actually be an iterator type. `priority_queue{1,2}`
should be SFINAE-friendly ill-formed.

Also, do a drive-by fix in the allocator-extended move constructor:
there's no need to do a `make_heap` after moving from `__q.c` into
our own `c`, because that container was already heapified when it
was part of `__q`. [priqueue.cons.alloc] actually specifies the
behavior and does *not* mention calling `make_heap`. I think this
was just a copy-paste thinko. It dates back to the initial import
of libc++.

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Jul 26 2021, 2:09 PM
Quuxplusone created this revision.
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 26 2021, 2:09 PM
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/ctor_iter_constraint.compile.pass.cpp
22

I had initially written this test as follows, to test SFINAE-friendliness; but during the rebase I temporarily forgot why I'd done that, and changed it to the much simpler (but not-testing-SFINAE) version you see above. If you want me to change back to the SFINAE version, I can.

template<class Seq>
std::true_type test(int, decltype(Seq(1,2))) { return {}; }

template<class Seq>
std::false_type test(long, Seq) { return {}; }

int main(int, char**)
{
    // Sanity-check that std::vector<int>(1,2) is well-formed.
    auto vector_is_constructible = test< std::vector<int> >(1, {});
    static_assert(decltype(vector_is_constructible)::value, "");

    // LWG3522: std::priority_queue<int>(1,2) is NOT well-formed.
    auto pq_is_constructible = test< std::priority_queue<int> >(1, {});
    static_assert(!decltype(pq_is_constructible)::value, "");

    return 0;
}
ldionne requested changes to this revision.Jul 26 2021, 2:14 PM
ldionne added inline comments.
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/ctor_iter_constraint.compile.pass.cpp
19

You're missing tests for some of the constructors you modified, aren't you?

This revision now requires changes to proceed.Jul 26 2021, 2:14 PM
Quuxplusone marked an inline comment as done.
ldionne accepted this revision.Jul 28 2021, 12:36 PM
This revision is now accepted and ready to land.Jul 28 2021, 12:36 PM
Quuxplusone retitled this revision from [libc++] Implement the resolution for LWG3522 in all language modes. to [libc++] Implement the resolutions for LWG3506 and LWG3522..
Quuxplusone edited the summary of this revision. (Show Details)

Roll D106824 into D106827. Add is_constructible tests also for the new constructors added in D106824.

Landed as part of rG3894a8a4768f

libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/ctor_iter_constraint.compile.pass.cpp
19

Yes; initially this was just a sanity-check to make sure that e.g.
std::priority_queue<int> pq = {1,2}; // or maybe more ints
didn't compile, even though it would compile fine for std::vector.
But adding the other signatures is easy, so done.