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. 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++.
Details
- Reviewers
ldionne mspertus tcanens - Group Reviewers
Restricted Project - Commits
- rG3894a8a4768f: [libc++] Implement the resolutions of LWG3506 and LWG3522.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
libcxx/include/queue | ||
---|---|---|
175 | Maybe we can get rid of the typename here? | |
537 | I'm not sure I understand why we're constraining those constructors with is_cpp17_input_iterator? I understand it's just generally a bad idea to have unconstrained template parameters, but that seems to be what the spec has? | |
554 | Doesn't this work even in C++03 mode with the rvalue references extension? | |
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons.alloc/ctor_iter_iter_comp_cont_alloc.pass.cpp | ||
21 | Why are you creating this derived type? | |
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.pass.cpp | ||
20 | This would need to be updated. |
libcxx/include/queue | ||
---|---|---|
537 | Never mind this, I just saw your other patch. |
Update tests: my sneaky inheritance of constructors didn't work in C++03 mode, so revert to something much closer to the existing approach in other priority_queue tests.
"13.0" is now "14.0".
libcxx/include/queue | ||
---|---|---|
554 | Yes, but I haven't been reckless enough yet to break our historical tradition that guards all actual container move-constructors and move-assignments under _LIBCPP_CXX03_LANG. I think we should break those all at once. Note particularly the priority_queue(const Comp&, Container&&, const Alloc&) on line 528, which is already guarded like this. | |
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons.alloc/ctor_iter_iter_comp_cont_alloc.pass.cpp | ||
21 | I'd been doing it in order to get at q.c.get_allocator() on line 35. |
libcxx/include/queue | ||
---|---|---|
554 | Oh, thanks, I had not realized there was such a tradition. |
LGTM, but can you please add SFINAE tests for the constructors that you added? I'd like to catch in case we had forgotten to constrain them properly (right now I don't think we would).
Maybe we can get rid of the typename here?