From cb0d2dad73d75befc7195073be64f53adb03863b Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 16 Nov 2023 00:34:10 +0800 Subject: [PATCH 1/3] Implement P2905R2 Runtime Format Strings (Make `make_(w)format_args` only take lvalue references) --- stl/inc/format | 14 ++++---- stl/inc/ostream | 6 ++-- stl/inc/print | 6 ++-- stl/inc/yvals_core.h | 1 + tests/libcxx/expected_results.txt | 10 ++++++ tests/std/include/test_format_support.hpp | 4 +-- .../test.cpp | 4 +-- .../P0645R10_text_formatting_args/test.cpp | 32 ++++++++++++++++++- .../test.cpp | 4 +-- .../test.cpp | 4 +-- .../P0645R10_text_formatting_utf8/test.cpp | 3 +- .../P2286R8_text_formatting_escaping/test.cpp | 4 +-- 12 files changed, 66 insertions(+), 26 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 105307688f..f509e1903a 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3824,18 +3824,20 @@ _EXPORT_STD using format_args = basic_format_args; _EXPORT_STD using wformat_args = basic_format_args; _EXPORT_STD template -_NODISCARD auto make_format_args(_Args&&... _Vals) { - static_assert((_Formattable_with, _Context> && ...), +_NODISCARD auto make_format_args(_Args&... _Vals) { + // TRANSITION, should cite the new working draft + static_assert((_Formattable_with, _Context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4950 [format.arg.store]/2 and [formatter.requirements]."); + "See N4964 [format.arg.store]/2 (along with modification in P2905R2) and [formatter.requirements]."); return _Format_arg_store<_Context, _Args...>{_Vals...}; } _EXPORT_STD template -_NODISCARD auto make_wformat_args(_Args&&... _Vals) { - static_assert((_Formattable_with, wformat_context> && ...), +_NODISCARD auto make_wformat_args(_Args&... _Vals) { + // TRANSITION, should cite the new working draft + static_assert((_Formattable_with, wformat_context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4950 [format.arg.store]/2 and [formatter.requirements]."); + "See N4964 [format.arg.store]/2 (along with modification in P2905R2) and [formatter.requirements]."); return _Format_arg_store{_Vals...}; } diff --git a/stl/inc/ostream b/stl/inc/ostream index de3b0a0fee..6e09a52c6d 100644 --- a/stl/inc/ostream +++ b/stl/inc/ostream @@ -1244,11 +1244,9 @@ void _Print_impl(const _Add_newline _Add_nl, ostream& _Ostr, const format_string if constexpr (_Has_format_args) { if constexpr (_STD _Is_ordinary_literal_encoding_utf8()) { - _STD _Vprint_unicode_impl( - _Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...)); + _STD _Vprint_unicode_impl(_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_Args...)); } else { - _STD _Vprint_nonunicode_impl( - _Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...)); + _STD _Vprint_nonunicode_impl(_Add_nl, _Ostr, _Fmt.get(), _STD make_format_args(_Args...)); } } else { const string _Unescaped_str{_Unescape_braces(_Add_nl, _Fmt.get())}; diff --git a/stl/inc/print b/stl/inc/print index 0e4c090925..312962f2f6 100644 --- a/stl/inc/print +++ b/stl/inc/print @@ -101,11 +101,9 @@ void _Print_impl( if constexpr (_Has_format_args) { if constexpr (_STD _Is_ordinary_literal_encoding_utf8()) { - _STD _Vprint_unicode_impl( - _Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...)); + _STD _Vprint_unicode_impl(_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_Args...)); } else { - _STD _Vprint_nonunicode_impl( - _Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_STD forward<_Types>(_Args)...)); + _STD _Vprint_nonunicode_impl(_Add_nl, _Stream, _Fmt.get(), _STD make_format_args(_Args...)); } } else { const string _Unescaped_str{_Unescape_braces(_Add_nl, _Fmt.get())}; diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 1c494ad110..6a06f98c52 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -306,6 +306,7 @@ // P2711R1 Making Multi-Param Constructors Of Views explicit // P2736R2 Referencing The Unicode Standard // P2770R0 Stashing Stashing Iterators For Proper Flattening +// P2905R2 Runtime Format Strings // _HAS_CXX20 indirectly controls: // P0619R4 Removing C++17-Deprecated Features diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 1763561685..7288e398c6 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -189,6 +189,16 @@ std/ranges/range.adaptors/range.join.view/sentinel/eq.pass.cpp FAIL std/re/re.iter/re.regiter/iterator_concept_conformance.compile.pass.cpp FAIL std/re/re.iter/re.tokiter/iterator_concept_conformance.compile.pass.cpp FAIL +# libc++ doesn't implement P2905R2 "Runtime Format Strings" +std/utilities/format/format.arguments/format.arg.store/make_format_args.pass.cpp FAIL +std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp FAIL +std/utilities/format/format.arguments/format.arg.store/make_wformat_args.pass.cpp FAIL +std/utilities/format/format.arguments/format.args/ctor.pass.cpp FAIL +std/utilities/format/format.formatter/format.context/format.context/arg.pass.cpp FAIL +std/utilities/format/format.formatter/format.context/format.context/ctor.pass.cpp FAIL +std/utilities/format/format.formatter/format.context/format.context/locale.pass.cpp FAIL +std/utilities/format/format.formatter/format.formatter.spec/formatter.string.pass.cpp FAIL + # *** INTERACTIONS WITH CONTEST / C1XX THAT UPSTREAM LIKELY WON'T FIX *** # Tracked by VSO-593630 " Enable libcxx filesystem tests" diff --git a/tests/std/include/test_format_support.hpp b/tests/std/include/test_format_support.hpp index c8b2a218f4..60ee59410c 100644 --- a/tests/std/include/test_format_support.hpp +++ b/tests/std/include/test_format_support.hpp @@ -161,9 +161,9 @@ struct VFormatFn { template [[nodiscard]] auto operator()(const std::basic_string_view str, Args&&... args) const { if constexpr (std::same_as) { - return std::vformat(str, std::make_format_args(std::forward(args)...)); + return std::vformat(str, std::make_format_args(args...)); } else { - return std::vformat(str, std::make_wformat_args(std::forward(args)...)); + return std::vformat(str, std::make_wformat_args(args...)); } } }; diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp index bf0baaa112..c897dd00be 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp @@ -215,9 +215,9 @@ bool test_parse_chrono_format_specs() { template auto make_testing_format_args(Args&&... vals) { if constexpr (is_same_v) { - return make_wformat_args(forward(vals)...); + return make_wformat_args(vals...); } else { - return make_format_args(forward(vals)...); + return make_format_args(vals...); } } diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 24ceead553..8ce7e762fe 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -248,7 +248,8 @@ void test_visit_monostate() { template void test_lwg3810() { - [[maybe_unused]] auto args_store = make_format_args(1, 2, 3); + int args[]{1, 2, 3}; + [[maybe_unused]] auto args_store = make_format_args(args[0], args[1], args[2]); static_assert(same_as>); } @@ -264,6 +265,32 @@ void test_lvalue_only_visitation() { visit_format_arg(lvalue_only_visitor{}, basic_format_arg{}); } +template +concept CanMakeFormatArgs = requires(Args&&... args) { make_format_args(static_cast(args)...); }; + +// P2905R2 Runtime format strings (make make_(w)format_args only take lvalue references) +template +void test_lvalue_reference_parameters() { // COMPILE-ONLY + using char_type = Context::char_type; + + static_assert(CanMakeFormatArgs&, basic_string_view&>); + static_assert( + CanMakeFormatArgs&, const basic_string_view&>); + + static_assert(CanMakeFormatArgs, const basic_string_view>); + + static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs>); + static_assert(!CanMakeFormatArgs>); +} + int main() { test_basic_format_arg(); test_basic_format_arg(); @@ -277,4 +304,7 @@ int main() { test_lvalue_only_visitation(); test_lvalue_only_visitation(); + + test_lvalue_reference_parameters(); + test_lvalue_reference_parameters(); } diff --git a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp index 928ff8aa89..4693ae2608 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -17,9 +17,9 @@ using namespace std; template auto make_testing_format_args(Args&&... vals) { if constexpr (is_same_v) { - return make_wformat_args(forward(vals)...); + return make_wformat_args(vals...); } else { - return make_format_args(forward(vals)...); + return make_format_args(vals...); } } diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index 9954c85d77..c009a0c002 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -65,9 +65,9 @@ using alternative_basic_string = basic_string auto make_testing_format_args(Args&&... vals) { if constexpr (is_same_v) { - return make_wformat_args(forward(vals)...); + return make_wformat_args(vals...); } else { - return make_format_args(forward(vals)...); + return make_format_args(vals...); } } diff --git a/tests/std/tests/P0645R10_text_formatting_utf8/test.cpp b/tests/std/tests/P0645R10_text_formatting_utf8/test.cpp index db01cedbef..c11d20e751 100644 --- a/tests/std/tests/P0645R10_text_formatting_utf8/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_utf8/test.cpp @@ -23,7 +23,8 @@ void test_multibyte_format_strings() { { try { // Bad fill character encoding: missing lead byte before \x9f - (void) vformat("{:\x9f\x8f\x88<10}"sv, make_format_args(42)); + int arg = 42; + (void) vformat("{:\x9f\x8f\x88<10}"sv, make_format_args(arg)); assert(false); } catch (const format_error&) { } diff --git a/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp b/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp index c735ff84da..33288777a5 100644 --- a/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp @@ -24,9 +24,9 @@ template template auto make_testing_format_args(Args&&... vals) { if constexpr (is_same_v) { - return make_wformat_args(forward(vals)...); + return make_wformat_args(vals...); } else { - return make_format_args(forward(vals)...); + return make_format_args(vals...); } } From 0e24b20c46c26124f4d7ed072c37a7df3cd9a760 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 16 Nov 2023 01:09:48 +0800 Subject: [PATCH 2/3] Handle `/permissive` --- .../P0645R10_text_formatting_args/test.cpp | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 8ce7e762fe..4244f9c8ad 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -265,6 +265,27 @@ void test_lvalue_only_visitation() { visit_format_arg(lvalue_only_visitor{}, basic_format_arg{}); } +namespace detail { + constexpr bool permissive() { + return false; + } + + template + struct DependentBase { + static constexpr bool permissive() { + return true; + } + }; + + template + struct Derived : DependentBase { + static constexpr bool test() { + return permissive(); + } + }; +} // namespace detail +constexpr bool is_permissive = detail::Derived::test(); + template concept CanMakeFormatArgs = requires(Args&&... args) { make_format_args(static_cast(args)...); }; @@ -287,8 +308,8 @@ void test_lvalue_reference_parameters() { // COMPILE-ONLY static_assert(!CanMakeFormatArgs); static_assert(!CanMakeFormatArgs); static_assert(!CanMakeFormatArgs); - static_assert(!CanMakeFormatArgs>); - static_assert(!CanMakeFormatArgs>); + static_assert(CanMakeFormatArgs> == is_permissive); + static_assert(CanMakeFormatArgs> == is_permissive); } int main() { From 03c2b358fc24d2c0abb670b01ded873180451f8e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 15 Dec 2023 17:39:23 -0800 Subject: [PATCH 3/3] Code review feedback. --- tests/libcxx/expected_results.txt | 14 -------------- .../test.cpp | 2 +- .../tests/P0645R10_text_formatting_args/test.cpp | 3 ++- .../test.cpp | 2 +- .../P0645R10_text_formatting_formatting/test.cpp | 2 +- .../P2286R8_text_formatting_escaping/test.cpp | 2 +- 6 files changed, 6 insertions(+), 19 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index dca8f7990f..4689556803 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -168,16 +168,6 @@ std/utilities/expected/expected.void/observers/value.pass.cpp FAIL # If any feature-test macro test is failing, this consolidated test will also fail. std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp FAIL -# libc++ doesn't implement P2905R2 "Runtime Format Strings" -std/utilities/format/format.arguments/format.arg.store/make_format_args.pass.cpp FAIL -std/utilities/format/format.arguments/format.arg.store/make_format_args.sh.cpp FAIL -std/utilities/format/format.arguments/format.arg.store/make_wformat_args.pass.cpp FAIL -std/utilities/format/format.arguments/format.args/ctor.pass.cpp FAIL -std/utilities/format/format.formatter/format.context/format.context/arg.pass.cpp FAIL -std/utilities/format/format.formatter/format.context/format.context/ctor.pass.cpp FAIL -std/utilities/format/format.formatter/format.context/format.context/locale.pass.cpp FAIL -std/utilities/format/format.formatter/format.formatter.spec/formatter.string.pass.cpp FAIL - # *** INTERACTIONS WITH CONTEST / C1XX THAT UPSTREAM LIKELY WON'T FIX *** # These tests set an allocator with a max_size() too small to default construct an unordered container @@ -318,10 +308,6 @@ std/language.support/support.limits/support.limits.general/utility.version.compi std/language.support/support.limits/support.limits.general/expected.version.compile.pass.cpp FAIL std/language.support/support.limits/support.limits.general/mdspan.version.compile.pass.cpp FAIL -# P2905R2 Runtime Format Strings -std/utilities/format/format.arguments/format.arg.store/make_format_args.pass.cpp FAIL -std/utilities/format/format.arguments/format.arg.store/make_wformat_args.pass.cpp FAIL - # P2918R2 Runtime Format Strings II std/utilities/format/format.fmt.string/ctor.runtime-format-string.pass.cpp FAIL std/utilities/format/format.functions/format.locale.runtime_format.pass.cpp FAIL diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp index 4c286aef7c..6b441e01ab 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp @@ -213,7 +213,7 @@ bool test_parse_chrono_format_specs() { } template -auto make_testing_format_args(Args&&... vals) { +auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful if constexpr (is_same_v) { return make_wformat_args(vals...); } else { diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 6d21791360..1af863a77a 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -291,7 +291,7 @@ concept CanMakeFormatArgs = requires(Args&&... args) { make_format_args // P2905R2 Runtime format strings (make make_(w)format_args only take lvalue references) template -void test_lvalue_reference_parameters() { // COMPILE-ONLY +void test_lvalue_reference_parameters() { using char_type = Context::char_type; static_assert(CanMakeFormatArgs); static_assert(!CanMakeFormatArgs); static_assert(!CanMakeFormatArgs); + static_assert(!CanMakeFormatArgs); static_assert(!CanMakeFormatArgs); static_assert(CanMakeFormatArgs> == is_permissive); static_assert(CanMakeFormatArgs> == is_permissive); diff --git a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp index cfd34240c8..5966daf359 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -15,7 +15,7 @@ using namespace std; // copied from the text_formatting_formatting test case template -auto make_testing_format_args(Args&&... vals) { +auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful if constexpr (is_same_v) { return make_wformat_args(vals...); } else { diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index 3669e57ac1..96c976e4be 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -63,7 +63,7 @@ template > using alternative_basic_string = basic_string, Alloc>; template -auto make_testing_format_args(Args&&... vals) { +auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful if constexpr (is_same_v) { return make_wformat_args(vals...); } else { diff --git a/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp b/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp index 33288777a5..19ffb64092 100644 --- a/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp @@ -22,7 +22,7 @@ template #define STR(Literal) (choose_literal(Literal, L##Literal)) template -auto make_testing_format_args(Args&&... vals) { +auto make_testing_format_args(Args&&... vals) { // references to temporaries are risky, see P2905R2; we'll be careful if constexpr (is_same_v) { return make_wformat_args(vals...); } else {