Uses-Allocator and guaranteed copy elision For piecewise construction #1668
Conversation
Co-Authored-By: ArtemSarmini <16746066+ArtemSarmini@users.noreply.github.com>
miscco
left a comment
miscco
left a comment
There was a problem hiding this comment.
I was todays years old as I learned about this feature.
It was a good time
Co-authored-by: Michael Schellenberger Costa <mschellenbergercosta@gmail.com>
|
Some libcxx tests needed to be disabled, because:
|
tests/std/tests/P0475R1_P0591R4_uses_allocator_construction/test.cpp
Outdated
Show resolved
Hide resolved
StephanTLavavej
left a comment
StephanTLavavej
left a comment
There was a problem hiding this comment.
This looks awesome! I exhaustively reviewed every line against the current WP, verifying that both papers and the relevant LWG issues, plus any later edits, are implemented. I also verified the correctness of the "equivalent to" transformations (which are a nice throughput improvement). Finally, it's great that _Uses_allocator_construct is no longer defined in C++20 mode. I'll push changes for extremely nitpicky comment issues and test variable names, then we'll get a final review and merge this! 🚀
tests/std/tests/P0475R1_P0591R4_uses_allocator_construction/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0475R1_P0591R4_uses_allocator_construction/test.cpp
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
CaseyCarter
left a comment
CaseyCarter
left a comment
There was a problem hiding this comment.
Only one comment, and it's actually a comment on the Standard and not this PR. This PR is great!
|
|
||
| #if _HAS_CXX20 | ||
| template <class _Ty, class _Alloc, class... _Types, enable_if_t<!_Is_specialization_v<_Ty, pair>, int> = 0> | ||
| _NODISCARD constexpr auto uses_allocator_construction_args(const _Alloc& _Al, _Types&&... _Args) noexcept { |
There was a problem hiding this comment.
Technically, these overloads of uses_allocator_construction_args are unimplementable since the Working Draft doesn't specify what types they return. (No change requested, just sharing my rage at the wording while I devise a curtly-worded issue report for LWG.)
There was a problem hiding this comment.
Clearly they can return anything :)
There was a problem hiding this comment.
"Implementing the unimplementable" is a nice slogan though 😹
There was a problem hiding this comment.
"Implementing the unimplementable" is a nice slogan though 😹
Ha! We should talk to @MahmoudGSaleh about getting some morale budget to have T-shirts made for all of our contributors.
|
Thanks @ArtemSarmini and @AdamBucior for implementing and testing these features! This will ship in VS 2019 16.10 Preview 2. 😻 🎉 🚀 |
5 participants
Based on #421.
Resolves #19 and resolves #20.
Most of this was implemented by @ArtemSarmini, I've just made some minor fixes and written test coverage.