From: Mark de Wever Date: Fri, 3 Mar 2023 19:23:51 +0000 (+0100) Subject: [libc++][format] Implements LWG3892. X-Git-Tag: upstream/17.0.6~12253 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9b43aedeb311576ee9c99d7a6317b5f697954dd9;p=platform%2Fupstream%2Fllvm.git [libc++][format] Implements LWG3892. This LWG issue is based on the discussion regarding P2733R3 Fix handling of empty specifiers in std::format This paper was disussed and changed a few times in LEWG during the Issaquah meeting. The paper was not voted in, instead LEWG asked for a DR against C++26. This LWG issue contains the direction voted by LEWG. This issue has not been voted in yet. However it fixes some of the defencies on the container based formatting. Without this fix the range-default-formatter for strings looks bad when used in containers. The changes of this issue match the intended changes of P27333. type fmt before after (if changed) --------------------------------------------------------------- char {} a char {:?} 'a' array {} ['a'] array {::} [a] array {::c} [a] array {::?} ['a'] map {} {a: a} -> {'a': 'a'} map {::} {'a': 'a'} set {} {'a'} set {::} {a} set {::c} {a} set {::?} {'a'} tuple {} ('a') stack {} ['a'] stack {::} [a] stack {::c} [a] stack {::?} ['a'] array, 1> {} [[a]] -> {'a': 'a'} array, 1> {::} [['a']] array, 1> {:::} [[a]] array, 1> {:::c} [[a]] array, 1> {:::?} [['a']] array, 1> {} [(a)] -> [('a')] tuple> {} ((a)) -> (('a')) tuple> {} ([a]) -> (['a']) Note the optimization text as mentioned in the tuple formatter can't be done. The call to parse may affect the formatter so its state needs to be preserved. Reviewed By: ldionne, #libc, EricWF Differential Revision: https://reviews.llvm.org/D145847 --- diff --git a/libcxx/docs/Status/Cxx2bIssues.csv b/libcxx/docs/Status/Cxx2bIssues.csv index f9cd4a1..99c83d8 100644 --- a/libcxx/docs/Status/Cxx2bIssues.csv +++ b/libcxx/docs/Status/Cxx2bIssues.csv @@ -308,3 +308,4 @@ "`3881 `__","Incorrect formatting of container adapters backed by ``std::string``","February 2023","|Complete|","17.0","|format|" "","","","","","" "`3343 `__","Ordering of calls to ``unlock()`` and ``notify_all()`` in Effects element of ``notify_all_at_thread_exit()`` should be reversed","Not Yet Adopted","|Complete|","16.0","" +"`3892 `__","Incorrect formatting of nested ranges and tuples","Not Yet Adopted","|Complete|","17.0","" diff --git a/libcxx/include/__format/formatter_tuple.h b/libcxx/include/__format/formatter_tuple.h index e6831de..9d6367b 100644 --- a/libcxx/include/__format/formatter_tuple.h +++ b/libcxx/include/__format/formatter_tuple.h @@ -53,33 +53,38 @@ struct _LIBCPP_TEMPLATE_VIS __formatter_tuple { _LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator parse(_ParseContext& __parse_ctx) { auto __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_tuple); - // [format.tuple]/7 - // ... For each element e in underlying_, if e.set_debug_format() - // is a valid expression, calls e.set_debug_format(). - // TODO FMT this can be removed when P2733 is accepted. - std::__for_each_index_sequence(make_index_sequence(), [&] { - std::__set_debug_format(std::get<_Index>(__underlying_)); - }); - auto __end = __parse_ctx.end(); - if (__begin == __end) - return __begin; - - if (*__begin == _CharT('m')) { - if constexpr (sizeof...(_Args) == 2) { - set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": ")); + if (__begin != __end) { + if (*__begin == _CharT('m')) { + if constexpr (sizeof...(_Args) == 2) { + set_separator(_LIBCPP_STATICALLY_WIDEN(_CharT, ": ")); + set_brackets({}, {}); + ++__begin; + } else + std::__throw_format_error("The format specifier m requires a pair or a two-element tuple"); + } else if (*__begin == _CharT('n')) { set_brackets({}, {}); ++__begin; - } else - std::__throw_format_error("The format specifier m requires a pair or a two-element tuple"); - } else if (*__begin == _CharT('n')) { - set_brackets({}, {}); - ++__begin; + } } if (__begin != __end && *__begin != _CharT('}')) std::__throw_format_error("The format-spec should consume the input or end with a '}'"); + __parse_ctx.advance_to(__begin); + + // [format.tuple]/7 + // ... For each element e in underlying_, if e.set_debug_format() + // is a valid expression, calls e.set_debug_format(). + std::__for_each_index_sequence(make_index_sequence(), [&] { + auto& __formatter = std::get<_Index>(__underlying_); + __formatter.parse(__parse_ctx); + // Unlike the range_formatter we don't guard against evil parsers. Since + // this format-spec never has a format-spec for the underlying type + // adding the test would give additional overhead. + std::__set_debug_format(__formatter); + }); + return __begin; } @@ -120,35 +125,7 @@ struct _LIBCPP_TEMPLATE_VIS __formatter_tuple { std::__for_each_index_sequence(make_index_sequence(), [&] { if constexpr (_Index) __ctx.advance_to(std::ranges::copy(__separator_, __ctx.out()).out); - - // During review Victor suggested to make the exposition only - // __underlying_ member a local variable. Currently the Standard - // requires nested debug-enabled formatter specializations not to - // output escaped output. P2733 fixes that bug, once accepted the - // code below can be used. - // (Note when a paper allows parsing a tuple-underlying-spec the - // exposition only member needs to be a class member. Earlier - // revisions of P2286 proposed that, but this was not pursued, - // due to time constrains and complexity of the matter.) - // TODO FMT This can be updated after P2733 is accepted. -# if 0 - // P2286 uses an exposition only member in the formatter - // tuple, _CharT>...> __underlying_; - // This was used in earlier versions of the paper since - // __underlying_.parse(...) was called. This is no longer the case - // so we can reduce the scope of the formatter. - // - // It does require the underlying's parse effect to be moved here too. - using _Arg = tuple_element<_Index, decltype(__tuple)>; - formatter, _CharT> __underlying; - - // [format.tuple]/7 - // ... For each element e in underlying_, if e.set_debug_format() - // is a valid expression, calls e.set_debug_format(). - std::__set_debug_format(__underlying); -# else __ctx.advance_to(std::get<_Index>(__underlying_).format(std::get<_Index>(__tuple), __ctx)); -# endif }); return std::ranges::copy(__closing_bracket_, __ctx.out()).out; diff --git a/libcxx/include/__format/range_formatter.h b/libcxx/include/__format/range_formatter.h index 4732343..0af233e 100644 --- a/libcxx/include/__format/range_formatter.h +++ b/libcxx/include/__format/range_formatter.h @@ -57,28 +57,57 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter { _LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator parse(_ParseContext& __parse_ctx) { auto __begin = __parser_.__parse(__parse_ctx, __format_spec::__fields_range); auto __end = __parse_ctx.end(); - if (__begin == __end) - return __begin; + // Note the cases where __begin == __end in this code only happens when the + // replacement-field has no terminating }, or when the parse is manually + // called with a format-spec. The former is an error and the latter means + // using a formatter without the format functions or print. + if (__begin == __end) [[unlikely]] + return __parse_empty_range_underlying_spec(__parse_ctx, __begin); // The n field overrides a possible m type, therefore delay applying the // effect of n until the type has been procesed. bool __clear_brackets = (*__begin == _CharT('n')); if (__clear_brackets) { ++__begin; - if (__begin == __end) { + if (__begin == __end) [[unlikely]] { // Since there is no more data, clear the brackets before returning. set_brackets({}, {}); - return __begin; + return __parse_empty_range_underlying_spec(__parse_ctx, __begin); } } __parse_type(__begin, __end); if (__clear_brackets) set_brackets({}, {}); - if (__begin == __end) - return __begin; + if (__begin == __end) [[unlikely]] + return __parse_empty_range_underlying_spec(__parse_ctx, __begin); bool __has_range_underlying_spec = *__begin == _CharT(':'); + if (__has_range_underlying_spec) { + // range-underlying-spec: + // : format-spec + ++__begin; + } else if (__begin != __end && *__begin != _CharT('}')) + // When there is no underlaying range the current parse should have + // consumed the format-spec. If not, the not consumed input will be + // processed by the underlying. For example {:-} for a range in invalid, + // the sign field is not present. Without this check the underlying_ will + // get -} as input which my be valid. + std::__throw_format_error("The format-spec should consume the input or end with a '}'"); + + __parse_ctx.advance_to(__begin); + __begin = __underlying_.parse(__parse_ctx); + + // This test should not be required if __has_range_underlying_spec is false. + // However this test makes sure the underlying formatter left the parser in + // a valid state. (Note this is not a full protection against evil parsers. + // For example + // } this is test for the next argument {} + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ + // could consume more than it should. + if (__begin != __end && *__begin != _CharT('}')) + std::__throw_format_error("The format-spec should consume the input or end with a '}'"); + if (__parser_.__type_ != __format_spec::__type::__default) { // [format.range.formatter]/6 // If the range-type is s or ?s, then there shall be no n option and no @@ -96,20 +125,6 @@ struct _LIBCPP_TEMPLATE_VIS range_formatter { } else if (!__has_range_underlying_spec) std::__set_debug_format(__underlying_); - if (__has_range_underlying_spec) { - // range-underlying-spec: - // : format-spec - ++__begin; - if (__begin == __end) - return __begin; - - __parse_ctx.advance_to(__begin); - __begin = __underlying_.parse(__parse_ctx); - } - - if (__begin != __end && *__begin != _CharT('}')) - std::__throw_format_error("The format-spec should consume the input or end with a '}'"); - return __begin; } @@ -243,6 +258,16 @@ private: } } + template + _LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator + __parse_empty_range_underlying_spec(_ParseContext& __parse_ctx, typename _ParseContext::iterator __begin) { + __parse_ctx.advance_to(__begin); + [[maybe_unused]] typename _ParseContext::iterator __result = __underlying_.parse(__parse_ctx); + _LIBCPP_ASSERT(__result == __begin, + "the underlying's parse function should not advance the input beyond the end of the input"); + return __begin; + } + formatter<_Tp, _CharT> __underlying_; basic_string_view<_CharT> __separator_ = _LIBCPP_STATICALLY_WIDEN(_CharT, ", "); basic_string_view<_CharT> __opening_bracket_ = _LIBCPP_STATICALLY_WIDEN(_CharT, "["); diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtdef/format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.fmtdef/format.pass.cpp index 8f39899..9c29722 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtdef/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtdef/format.pass.cpp @@ -26,6 +26,8 @@ #include #include +#include "assert_macros.h" +#include "format.functions.common.h" #include "test_format_context.h" #include "test_macros.h" #include "make_string.h" @@ -49,9 +51,50 @@ void test_format(StringViewT expected, std::array arg) { } template +void test_assure_parse_is_called(std::basic_string_view fmt) { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::array arg; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + std::formatter formatter; + std::basic_format_parse_context ctx{fmt}; + + formatter.parse(ctx); + formatter.format(arg, format_ctx); +} + +template +void test_assure_parse_is_called() { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::array arg; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + { // parse not called + [[maybe_unused]] const std::formatter formatter; + TEST_THROWS_TYPE(parse_call_validator::parse_function_not_called, formatter.format(arg, format_ctx)); + } + + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("n")); + test_assure_parse_is_called(SV("5n")); +} + +template void test_fmt() { test_format(SV("[1, 42]"), std::array{{1, 42}}); test_format(SV("[0, 99]"), std::array{{0, 99}}); + + test_assure_parse_is_called(); } void test() { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.format.pass.cpp index b6d562a..2dcdfc8 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.format.pass.cpp @@ -37,7 +37,7 @@ auto test = []( std::basic_string out = std::format(fmt, std::forward(args)...); TEST_REQUIRE(out == expected, TEST_WRITE_CONCATENATED( - "\nFormat string ", fmt, "\nExpected output ", expected, "\nActual output ", out, '\n')); + "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); }; auto test_exception = [](std::string_view, std::basic_string_view, Args&&...) { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.tests.h b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.tests.h index 005a2bd..3aa8953 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.tests.h +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.functions.tests.h @@ -26,20 +26,20 @@ template void test_char(TestFunction check, ExceptionTest check_exception) { std::map input{{CharT('a'), CharT('A')}, {CharT('c'), CharT('C')}, {CharT('b'), CharT('B')}}; - check(SV("{a: A, b: B, c: C}"), SV("{}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{}"), input); // ***** underlying has no format-spec // *** align-fill & width *** - check(SV("{a: A, b: B, c: C} "), SV("{:23}"), input); - check(SV("{a: A, b: B, c: C}*****"), SV("{:*<23}"), input); - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^23}"), input); - check(SV("#####{a: A, b: B, c: C}"), SV("{:#>23}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'} "), SV("{:35}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}*****"), SV("{:*<35}"), input); + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^35}"), input); + check(SV("#####{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{:#>35}"), input); - check(SV("{a: A, b: B, c: C} "), SV("{:{}}"), input, 23); - check(SV("{a: A, b: B, c: C}*****"), SV("{:*<{}}"), input, 23); - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^{}}"), input, 23); - check(SV("#####{a: A, b: B, c: C}"), SV("{:#>{}}"), input, 23); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'} "), SV("{:{}}"), input, 35); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}*****"), SV("{:*<{}}"), input, 35); + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^{}}"), input, 35); + check(SV("#####{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{:#>{}}"), input, 35); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -63,10 +63,10 @@ void test_char(TestFunction check, ExceptionTest check_exception) { check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** n - check(SV("__a: A, b: B, c: C___"), SV("{:_^21n}"), input); + check(SV("__'a': 'A', 'b': 'B', 'c': 'C'___"), SV("{:_^33n}"), input); // *** type *** - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^23m}"), input); // the m type does the same as the default. + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^35m}"), input); // the m type does the same as the default. check_exception("The range-format-spec type s requires formatting a character type", SV("{:s}"), input); check_exception("The range-format-spec type ?s requires formatting a character type", SV("{:?s}"), input); @@ -134,20 +134,20 @@ void test_char_to_wchar(TestFunction check, ExceptionTest check_exception) { std::map input{{'a', 'A'}, {'c', 'C'}, {'b', 'B'}}; using CharT = wchar_t; - check(SV("{a: A, b: B, c: C}"), SV("{}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{}"), input); // ***** underlying has no format-spec // *** align-fill & width *** - check(SV("{a: A, b: B, c: C} "), SV("{:23}"), input); - check(SV("{a: A, b: B, c: C}*****"), SV("{:*<23}"), input); - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^23}"), input); - check(SV("#####{a: A, b: B, c: C}"), SV("{:#>23}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'} "), SV("{:35}"), input); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}*****"), SV("{:*<35}"), input); + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^35}"), input); + check(SV("#####{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{:#>35}"), input); - check(SV("{a: A, b: B, c: C} "), SV("{:{}}"), input, 23); - check(SV("{a: A, b: B, c: C}*****"), SV("{:*<{}}"), input, 23); - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^{}}"), input, 23); - check(SV("#####{a: A, b: B, c: C}"), SV("{:#>{}}"), input, 23); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'} "), SV("{:{}}"), input, 35); + check(SV("{'a': 'A', 'b': 'B', 'c': 'C'}*****"), SV("{:*<{}}"), input, 35); + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^{}}"), input, 35); + check(SV("#####{'a': 'A', 'b': 'B', 'c': 'C'}"), SV("{:#>{}}"), input, 35); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -171,10 +171,10 @@ void test_char_to_wchar(TestFunction check, ExceptionTest check_exception) { check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** n - check(SV("__a: A, b: B, c: C___"), SV("{:_^21n}"), input); + check(SV("__'a': 'A', 'b': 'B', 'c': 'C'___"), SV("{:_^33n}"), input); // *** type *** - check(SV("__{a: A, b: B, c: C}___"), SV("{:_^23m}"), input); // the m type does the same as the default. + check(SV("__{'a': 'A', 'b': 'B', 'c': 'C'}___"), SV("{:_^35m}"), input); // the m type does the same as the default. check_exception("The range-format-spec type s requires formatting a character type", SV("{:s}"), input); check_exception("The range-format-spec type ?s requires formatting a character type", SV("{:?s}"), input); @@ -642,20 +642,20 @@ void test_string(TestFunction check, ExceptionTest check_exception) { std::map, std::basic_string> input{ {STR("hello"), STR("HELLO")}, {STR("world"), STR("WORLD")}}; - check(SV(R"({hello: HELLO, world: WORLD})"), SV("{}"), input); + check(SV(R"({"hello": "HELLO", "world": "WORLD"})"), SV("{}"), input); // ***** underlying has no format-spec // *** align-fill & width *** - check(SV(R"({hello: HELLO, world: WORLD} )"), SV("{:33}"), input); - check(SV(R"({hello: HELLO, world: WORLD}*****)"), SV("{:*<33}"), input); - check(SV(R"(__{hello: HELLO, world: WORLD}___)"), SV("{:_^33}"), input); - check(SV(R"(#####{hello: HELLO, world: WORLD})"), SV("{:#>33}"), input); + check(SV(R"({"hello": "HELLO", "world": "WORLD"} )"), SV("{:41}"), input); + check(SV(R"({"hello": "HELLO", "world": "WORLD"}*****)"), SV("{:*<41}"), input); + check(SV(R"(__{"hello": "HELLO", "world": "WORLD"}___)"), SV("{:_^41}"), input); + check(SV(R"(#####{"hello": "HELLO", "world": "WORLD"})"), SV("{:#>41}"), input); - check(SV(R"({hello: HELLO, world: WORLD} )"), SV("{:{}}"), input, 33); - check(SV(R"({hello: HELLO, world: WORLD}*****)"), SV("{:*<{}}"), input, 33); - check(SV(R"(__{hello: HELLO, world: WORLD}___)"), SV("{:_^{}}"), input, 33); - check(SV(R"(#####{hello: HELLO, world: WORLD})"), SV("{:#>{}}"), input, 33); + check(SV(R"({"hello": "HELLO", "world": "WORLD"} )"), SV("{:{}}"), input, 41); + check(SV(R"({"hello": "HELLO", "world": "WORLD"}*****)"), SV("{:*<{}}"), input, 41); + check(SV(R"(__{"hello": "HELLO", "world": "WORLD"}___)"), SV("{:_^{}}"), input, 41); + check(SV(R"(#####{"hello": "HELLO", "world": "WORLD"})"), SV("{:#>{}}"), input, 41); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -677,10 +677,10 @@ void test_string(TestFunction check, ExceptionTest check_exception) { check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** n - check(SV(R"(__hello: HELLO, world: WORLD___)"), SV("{:_^31n}"), input); + check(SV(R"(__"hello": "HELLO", "world": "WORLD"___)"), SV("{:_^39n}"), input); // *** type *** - check(SV(R"(__{hello: HELLO, world: WORLD}___)"), SV("{:_^33m}"), input); + check(SV(R"(__{"hello": "HELLO", "world": "WORLD"}___)"), SV("{:_^41m}"), input); check_exception("The range-format-spec type s requires formatting a character type", SV("{:s}"), input); check_exception("The range-format-spec type ?s requires formatting a character type", SV("{:?s}"), input); diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.pass.cpp index 2275bae..8ac3220a 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtmap/format.pass.cpp @@ -29,6 +29,8 @@ #include #include +#include "assert_macros.h" +#include "format.functions.common.h" #include "test_format_context.h" #include "test_macros.h" #include "make_string.h" @@ -52,9 +54,52 @@ void test_format(StringViewT expected, std::map arg) { } template +void test_assure_parse_is_called(std::basic_string_view fmt) { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::map arg{{parse_call_validator{}, parse_call_validator{}}}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + std::formatter formatter; + std::basic_format_parse_context ctx{fmt}; + + formatter.parse(ctx); + formatter.format(arg, format_ctx); +} + +template +void test_assure_parse_is_called() { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::map arg{{parse_call_validator{}, parse_call_validator{}}}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + { // parse not called + [[maybe_unused]] const std::formatter formatter; + TEST_THROWS_TYPE(parse_call_validator::parse_function_not_called, formatter.format(arg, format_ctx)); + } + + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("n")); + test_assure_parse_is_called(SV("m")); + test_assure_parse_is_called(SV("5n")); + test_assure_parse_is_called(SV("5m")); +} + +template void test_fmt() { test_format(SV("{1: 42}"), std::map{{1, 42}}); test_format(SV("{0: 99}"), std::map{{0, 99}}); + + test_assure_parse_is_called(); } void test() { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.format.pass.cpp index 10d5204..8272d13 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.format.pass.cpp @@ -37,7 +37,7 @@ auto test = []( std::basic_string out = std::format(fmt, std::forward(args)...); TEST_REQUIRE(out == expected, TEST_WRITE_CONCATENATED( - "\nFormat string ", fmt, "\nExpected output ", expected, "\nActual output ", out, '\n')); + "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); }; auto test_exception = [](std::string_view, std::basic_string_view, Args&&...) { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.tests.h b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.tests.h index cd0bc5a..bbb49e2 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.tests.h +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.functions.tests.h @@ -1100,20 +1100,20 @@ void test_status(TestFunction check, ExceptionTest check_exception) { template void test_pair_tuple(TestFunction check, ExceptionTest check_exception, auto&& input) { - check(SV("{(1, a), (42, *)}"), SV("{}"), input); + check(SV("{(1, 'a'), (42, '*')}"), SV("{}"), input); // ***** underlying has no format-spec // *** align-fill & width *** - check(SV("{(1, a), (42, *)} "), SV("{:22}"), input); - check(SV("{(1, a), (42, *)}*****"), SV("{:*<22}"), input); - check(SV("__{(1, a), (42, *)}___"), SV("{:_^22}"), input); - check(SV("#####{(1, a), (42, *)}"), SV("{:#>22}"), input); + check(SV("{(1, 'a'), (42, '*')} "), SV("{:26}"), input); + check(SV("{(1, 'a'), (42, '*')}*****"), SV("{:*<26}"), input); + check(SV("__{(1, 'a'), (42, '*')}___"), SV("{:_^26}"), input); + check(SV("#####{(1, 'a'), (42, '*')}"), SV("{:#>26}"), input); - check(SV("{(1, a), (42, *)} "), SV("{:{}}"), input, 22); - check(SV("{(1, a), (42, *)}*****"), SV("{:*<{}}"), input, 22); - check(SV("__{(1, a), (42, *)}___"), SV("{:_^{}}"), input, 22); - check(SV("#####{(1, a), (42, *)}"), SV("{:#>{}}"), input, 22); + check(SV("{(1, 'a'), (42, '*')} "), SV("{:{}}"), input, 26); + check(SV("{(1, 'a'), (42, '*')}*****"), SV("{:*<{}}"), input, 26); + check(SV("__{(1, 'a'), (42, '*')}___"), SV("{:_^{}}"), input, 26); + check(SV("#####{(1, 'a'), (42, '*')}"), SV("{:#>{}}"), input, 26); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -1137,11 +1137,11 @@ void test_pair_tuple(TestFunction check, ExceptionTest check_exception, auto&& i check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** n - check(SV("__(1, a), (42, *)___"), SV("{:_^20n}"), input); - check(SV("__(1, a), (42, *)___"), SV("{:_^20nm}"), input); // m should have no effect + check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24n}"), input); + check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24nm}"), input); // m should have no effect // *** type *** - check(SV("__{(1, a), (42, *)}___"), SV("{:_^22m}"), input); + check(SV("__{(1, 'a'), (42, '*')}___"), SV("{:_^26m}"), input); check_exception("The range-format-spec type s requires formatting a character type", SV("{:s}"), input); check_exception("The range-format-spec type ?s requires formatting a character type", SV("{:?s}"), input); diff --git a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.pass.cpp index dcb3d67..2da3c7f 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.fmtset/format.pass.cpp @@ -29,6 +29,8 @@ #include #include +#include "assert_macros.h" +#include "format.functions.common.h" #include "test_format_context.h" #include "test_macros.h" #include "make_string.h" @@ -52,8 +54,49 @@ void test_format(StringViewT expected, std::set arg) { } template +void test_assure_parse_is_called(std::basic_string_view fmt) { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::set arg{parse_call_validator{}}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + std::formatter formatter; + std::basic_format_parse_context ctx{fmt}; + + formatter.parse(ctx); + formatter.format(arg, format_ctx); +} + +template +void test_assure_parse_is_called() { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::set arg{parse_call_validator{}}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + { // parse not called + [[maybe_unused]] const std::formatter formatter; + TEST_THROWS_TYPE(parse_call_validator::parse_function_not_called, formatter.format(arg, format_ctx)); + } + + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("n")); + test_assure_parse_is_called(SV("5n")); +} + +template void test_fmt() { test_format(SV("{42}"), std::set{42}); + + test_assure_parse_is_called(); } void test() { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.format.pass.cpp index 21d80a5..02c9467 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.format.pass.cpp @@ -38,7 +38,7 @@ auto test = []( std::basic_string out = std::format(fmt, std::forward(args)...); TEST_REQUIRE(out == expected, TEST_WRITE_CONCATENATED( - "\nFormat string ", fmt, "\nExpected output ", expected, "\nActual output ", out, '\n')); + "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); }; auto test_exception = [](std::string_view, std::basic_string_view, Args&&...) { diff --git a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h index 28ff1cb..57f1861 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h +++ b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h @@ -928,20 +928,20 @@ void test_pair_tuple(TestFunction check, ExceptionTest check_exception, auto&& i // So when there is no range-underlying-spec, there is no need to call parse // thus the char element is not escaped. // TODO FMT P2733 addresses this issue. - check(SV("[(1, a), (42, *)]"), SV("{}"), input); + check(SV("[(1, 'a'), (42, '*')]"), SV("{}"), input); // ***** underlying has no format-spec // *** align-fill & width *** - check(SV("[(1, a), (42, *)] "), SV("{:22}"), input); - check(SV("[(1, a), (42, *)]*****"), SV("{:*<22}"), input); - check(SV("__[(1, a), (42, *)]___"), SV("{:_^22}"), input); - check(SV("#####[(1, a), (42, *)]"), SV("{:#>22}"), input); + check(SV("[(1, 'a'), (42, '*')] "), SV("{:26}"), input); + check(SV("[(1, 'a'), (42, '*')]*****"), SV("{:*<26}"), input); + check(SV("__[(1, 'a'), (42, '*')]___"), SV("{:_^26}"), input); + check(SV("#####[(1, 'a'), (42, '*')]"), SV("{:#>26}"), input); - check(SV("[(1, a), (42, *)] "), SV("{:{}}"), input, 22); - check(SV("[(1, a), (42, *)]*****"), SV("{:*<{}}"), input, 22); - check(SV("__[(1, a), (42, *)]___"), SV("{:_^{}}"), input, 22); - check(SV("#####[(1, a), (42, *)]"), SV("{:#>{}}"), input, 22); + check(SV("[(1, 'a'), (42, '*')] "), SV("{:{}}"), input, 26); + check(SV("[(1, 'a'), (42, '*')]*****"), SV("{:*<{}}"), input, 26); + check(SV("__[(1, 'a'), (42, '*')]___"), SV("{:_^{}}"), input, 26); + check(SV("#####[(1, 'a'), (42, '*')]"), SV("{:#>{}}"), input, 26); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -965,11 +965,11 @@ void test_pair_tuple(TestFunction check, ExceptionTest check_exception, auto&& i check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** n - check(SV("__(1, a), (42, *)___"), SV("{:_^20n}"), input); - check(SV("__(1, a), (42, *)___"), SV("{:_^20nm}"), input); // m should have no effect + check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24n}"), input); + check(SV("__(1, 'a'), (42, '*')___"), SV("{:_^24nm}"), input); // m should have no effect // *** type *** - check(SV("__{(1, a), (42, *)}___"), SV("{:_^22m}"), input); + check(SV("__{(1, 'a'), (42, '*')}___"), SV("{:_^26m}"), input); check_exception("The range-format-spec type s requires formatting a character type", SV("{:s}"), input); check_exception("The range-format-spec type ?s requires formatting a character type", SV("{:?s}"), input); for (std::basic_string_view fmt : fmt_invalid_types("s")) diff --git a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.pass.cpp b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.pass.cpp index faf9e1a..e62c562 100644 --- a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.pass.cpp @@ -30,9 +30,11 @@ #include #include +#include "assert_macros.h" +#include "format.functions.common.h" +#include "make_string.h" #include "test_format_context.h" #include "test_macros.h" -#include "make_string.h" #define SV(S) MAKE_STRING_VIEW(CharT, S) @@ -53,9 +55,55 @@ void test_format(StringViewT expected, std::vector arg) { } template +void test_assure_parse_is_called(std::basic_string_view fmt) { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::vector arg{1}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + std::range_formatter formatter; + std::basic_format_parse_context ctx{fmt}; + + formatter.parse(ctx); + formatter.format(arg, format_ctx); +} + +template +void test_assure_parse_is_called() { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::vector arg{1}; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + { // parse not called + [[maybe_unused]] const std::range_formatter formatter; + TEST_THROWS_TYPE(parse_call_validator::parse_function_not_called, formatter.format(arg, format_ctx)); + } + + // The range-format-spec has no range-underlying-spec + // This uses various variants, which have different code paths in libc++ + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("n")); + test_assure_parse_is_called(SV(":")); + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("5n")); + test_assure_parse_is_called(SV("5n:")); +} + +template void test_fmt() { test_format(SV("[1]"), std::vector{1}); test_format(SV("[0]"), std::vector{0}); + + test_assure_parse_is_called(); } void test() { diff --git a/libcxx/test/std/utilities/format/format.tuple/format.functions.format.pass.cpp b/libcxx/test/std/utilities/format/format.tuple/format.functions.format.pass.cpp index 7a404d8..1132815 100644 --- a/libcxx/test/std/utilities/format/format.tuple/format.functions.format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.tuple/format.functions.format.pass.cpp @@ -40,7 +40,7 @@ auto test = []( std::basic_string out = std::format(fmt, std::forward(args)...); TEST_REQUIRE(out == expected, TEST_WRITE_CONCATENATED( - "\nFormat string ", fmt, "\nExpected output ", expected, "\nActual output ", out, '\n')); + "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); }; auto test_exception = [](std::string_view, std::basic_string_view, Args&&...) { diff --git a/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h b/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h index ab401d3..6ebc17d 100644 --- a/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h +++ b/libcxx/test/std/utilities/format/format.tuple/format.functions.tests.h @@ -288,18 +288,18 @@ void test_nested(TestFunction check, ExceptionTest check_exception, Nested&& inp // P2733 Fix handling of empty specifiers in std::format // addressed this. - check(SV("(42, (hello, red))"), SV("{}"), input); + check(SV("(42, (\"hello\", \"red\"))"), SV("{}"), input); // *** align-fill & width *** - check(SV("(42, (hello, red)) "), SV("{:23}"), input); - check(SV("(42, (hello, red))*****"), SV("{:*<23}"), input); - check(SV("__(42, (hello, red))___"), SV("{:_^23}"), input); - check(SV("#####(42, (hello, red))"), SV("{:#>23}"), input); + check(SV("(42, (\"hello\", \"red\")) "), SV("{:27}"), input); + check(SV("(42, (\"hello\", \"red\"))*****"), SV("{:*<27}"), input); + check(SV("__(42, (\"hello\", \"red\"))___"), SV("{:_^27}"), input); + check(SV("#####(42, (\"hello\", \"red\"))"), SV("{:#>27}"), input); - check(SV("(42, (hello, red)) "), SV("{:{}}"), input, 23); - check(SV("(42, (hello, red))*****"), SV("{:*<{}}"), input, 23); - check(SV("__(42, (hello, red))___"), SV("{:_^{}}"), input, 23); - check(SV("#####(42, (hello, red))"), SV("{:#>{}}"), input, 23); + check(SV("(42, (\"hello\", \"red\")) "), SV("{:{}}"), input, 27); + check(SV("(42, (\"hello\", \"red\"))*****"), SV("{:*<{}}"), input, 27); + check(SV("__(42, (\"hello\", \"red\"))___"), SV("{:_^{}}"), input, 27); + check(SV("#####(42, (\"hello\", \"red\"))"), SV("{:#>{}}"), input, 27); check_exception("The format-spec range-fill field contains an invalid character", SV("{:}<}"), input); check_exception("The format-spec range-fill field contains an invalid character", SV("{:{<}"), input); @@ -323,8 +323,8 @@ void test_nested(TestFunction check, ExceptionTest check_exception, Nested&& inp check_exception("The format-spec should consume the input or end with a '}'", SV("{:L}"), input); // *** type *** - check(SV("__42: (hello, red)___"), SV("{:_^21m}"), input); - check(SV("__42, (hello, red)___"), SV("{:_^21n}"), input); + check(SV("__42: (\"hello\", \"red\")___"), SV("{:_^25m}"), input); + check(SV("__42, (\"hello\", \"red\")___"), SV("{:_^25n}"), input); for (CharT c : SV("aAbBcdeEfFgGopsxX?")) { check_exception("The format-spec should consume the input or end with a '}'", diff --git a/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp b/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp index 2d2e60c..d9571e6 100644 --- a/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp @@ -30,9 +30,11 @@ #include #include +#include "assert_macros.h" +#include "format.functions.common.h" +#include "make_string.h" #include "test_format_context.h" #include "test_macros.h" -#include "make_string.h" #define SV(S) MAKE_STRING_VIEW(CharT, S) @@ -53,11 +55,54 @@ void test(StringViewT expected, Arg arg) { } template +void test_assure_parse_is_called(std::basic_string_view fmt) { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::pair arg; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + std::formatter formatter; + std::basic_format_parse_context ctx{fmt}; + + formatter.parse(ctx); + formatter.format(arg, format_ctx); +} + +template +void test_assure_parse_is_called() { + using String = std::basic_string; + using OutIt = std::back_insert_iterator; + using FormatCtxT = std::basic_format_context; + std::pair arg; + + String result; + OutIt out = std::back_inserter(result); + FormatCtxT format_ctx = test_format_context_create(out, std::make_format_args(arg)); + + { // parse not called + [[maybe_unused]] const std::formatter formatter; + TEST_THROWS_TYPE(parse_call_validator::parse_function_not_called, formatter.format(arg, format_ctx)); + } + + test_assure_parse_is_called(SV("5")); + test_assure_parse_is_called(SV("n")); + test_assure_parse_is_called(SV("m")); + test_assure_parse_is_called(SV("5n")); + test_assure_parse_is_called(SV("5m")); +} + +template void test() { test(SV("(1)"), std::tuple{1}); test(SV("(1, 1)"), std::tuple{1, CharT('1')}); test(SV("(1, 1)"), std::pair{1, CharT('1')}); test(SV("(1, 1, true)"), std::tuple{1, CharT('1'), true}); + + test_assure_parse_is_called(); } void test() { diff --git a/libcxx/test/support/format.functions.common.h b/libcxx/test/support/format.functions.common.h index 65da5fd..e5f6bbb 100644 --- a/libcxx/test/support/format.functions.common.h +++ b/libcxx/test/support/format.functions.common.h @@ -50,11 +50,17 @@ enum class status : std::uint16_t { foo = 0xAAAA, bar = 0x5555, foobar = 0xAA55 // The formatter for a user-defined type used to test the handle formatter. template struct std::formatter { - int type = 0; + // During the 2023 Issaquah meeting LEWG made it clear a formatter is + // required to call its parse function. LWG3892 Adds the wording for that + // requirement. Therefore this formatter is initialized in an invalid state. + // A call to parse sets it in a valid state and a call to format validates + // the state. + int type = -1; constexpr auto parse(basic_format_parse_context& parse_ctx) -> decltype(parse_ctx.begin()) { auto begin = parse_ctx.begin(); - auto end = parse_ctx.end(); + auto end = parse_ctx.end(); + type = 0; if (begin == end) return begin; @@ -87,6 +93,9 @@ struct std::formatter { const char* begin = names[0]; const char* end = names[0]; switch (type) { + case -1: + throw_format_error("The formatter's parse function has not been called."); + case 0: begin = buffer; buffer[0] = '0'; @@ -125,7 +134,7 @@ struct std::formatter { } private: - void throw_format_error(const char* s) { + void throw_format_error(const char* s) const { #ifndef TEST_HAS_NO_EXCEPTIONS throw std::format_error(s); #else @@ -135,6 +144,52 @@ private: } }; +struct parse_call_validator { + struct parse_function_not_called {}; + + friend constexpr auto operator<=>(const parse_call_validator& lhs, const parse_call_validator& rhs) { + return &lhs <=> &rhs; + } +}; + +// The formatter for a user-defined type used to test the handle formatter. +// +// Like std::formatter this formatter validates that parse is +// called. This formatter is intended to be used when the formatter's parse is +// called directly and not with format. In that case the format-spec does not +// require a terminating }. The tests must be written in a fashion where this +// formatter is always called with an empty format-spec. This requirement +// allows testing of certain code paths that are never reached by using a +// well-formed format-string in the format functions. +template +struct std::formatter { + bool parse_called{false}; + + constexpr auto parse(basic_format_parse_context& parse_ctx) -> decltype(parse_ctx.begin()) { + auto begin = parse_ctx.begin(); + auto end = parse_ctx.end(); + assert(begin == end); + parse_called = true; + return begin; + } + + auto format(parse_call_validator, auto& ctx) const -> decltype(ctx.out()) { + if (!parse_called) + throw_error(); + return ctx.out(); + } + +private: + template + [[noreturn]] void throw_error() const { +#ifndef TEST_HAS_NO_EXCEPTIONS + throw T{}; +#else + std::abort(); +#endif + } +}; + // Creates format string for the invalid types. // // valid contains a list of types that are valid.