<format>: Implement P2418R2 support for std::generator-like types#2323
<format>: Implement P2418R2 support for std::generator-like types#2323StephanTLavavej merged 14 commits intomicrosoft:mainfrom
std::generator-like types#2323Conversation
std::generator-like types
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
…ing that it not const formattable.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
Also include <utility> for forward().
|
Thanks! I went ahead and pushed a batch of changes to address my feedback after running all tests. |
stl/inc/format
Outdated
| _NODISCARD auto make_format_args(const _Args&... _Vals) { | ||
| return _Format_arg_store<_Context, _Args...>{_Vals...}; | ||
| _NODISCARD auto make_format_args(_Args&&... _Vals) { | ||
| return _Format_arg_store<_Context, _Args...>{_STD forward<_Args>(_Vals)...}; |
There was a problem hiding this comment.
Why are we forwarding when the Working Draft doesn't show forwarding? (Also on 2966.) If we don't forward here, the _Format_arg_store constructor can/should be changed to take only lvalues.
Have you submitted an LWG issue to fix the preconditions on these functions to strip cvref-quals from the Ti?
There was a problem hiding this comment.
I'm forwarding because libfmt forwards here, I think you're right though and they forward everywhere but then specifically undo what the forwarding would have done when make_arg binds them all to the fmt equivalent to const _Storage_type<remove_cvref_t<T>>&
Have you submitted an LWG issue to fix the preconditions on these functions to strip cvref-quals from the Ti?
no, but Tim Song did.
There was a problem hiding this comment.
wait is this a separate issue from LWG-3631?
There was a problem hiding this comment.
Yes it is - [format.arg.store]/2. We can do this as a followup - it doesn't need to block merging.
tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp
Outdated
Show resolved
Hide resolved
|
I pushed a conflict-free merge with |
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for implementing the last C++20 DR! 🎉 😻 ✅ |
Implements P2418R2 which prepares format to be able to format std::generators, by allowing "format" functions that take their parameters by non-const reference. This DR is somewhat high risk because it modifies the argument type mapping machinery.
_Storage_typeto useremove_cvref_t<T>instead ofTbecause that's what the "other end" (the un-erase from handle) uses now. I don't think this has actual effect on the mapping, but could change the selected formatter specialization for custom types. Tim Song indicated on the reflectors that the lack ofremove_cvref_t<T>in [format.arg]/5 is probably a defect._Has_formatterand the new_Has_const_formatterconcepts have been moved up since they are now needed in the constructor forbasic_format_arg::handlehttp://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2418r2.html
Fixes #2239.