diff --git a/libcxx/docs/Status/Cxx2bIssues.csv b/libcxx/docs/Status/Cxx2bIssues.csv --- 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 --- a/libcxx/include/__format/formatter_tuple.h +++ b/libcxx/include/__format/formatter_tuple.h @@ -53,33 +53,38 @@ _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 @@ 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 --- a/libcxx/include/__format/range_formatter.h +++ b/libcxx/include/__format/range_formatter.h @@ -57,28 +57,57 @@ _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 @@ } 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 @@ } } + template + _LIBCPP_HIDE_FROM_ABI constexpr typename _ParseContext::iterator + __parse_empty_range_underlying_spec(_ParseContext& __parse_ctx, _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 --- 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 @@ -30,6 +30,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,10 +54,51 @@ assert(result == expected); } +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 --- 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,9 +37,10 @@ auto test = []( std::basic_string_view expected, test_format_string fmt, Args&&... args) { 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')); + TEST_REQUIRE( + out == expected, + TEST_WRITE_CONCATENATED( + "\nFormat string ", fmt.get(), "\nExpected output ", expected, "\nActual output ", out, '\n')); // XXX }; 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 --- 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 --- 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 @@ -33,6 +33,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" @@ -55,10 +57,53 @@ assert(result == expected); } +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 --- 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 @@ -39,7 +39,7 @@ 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 --- 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 @@ 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 @@ 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 --- 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 @@ -33,6 +33,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" @@ -55,9 +57,50 @@ assert(result == expected); } +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 --- 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 @@ -40,7 +40,7 @@ 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 --- 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 @@ // 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 @@ 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 --- 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 @@ -34,9 +34,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) @@ -56,10 +58,56 @@ assert(result == expected); } +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 --- 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 @@ -41,7 +41,7 @@ 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 --- 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 @@ // 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 @@ 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 --- a/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp +++ b/libcxx/test/std/utilities/format/format.tuple/format.pass.cpp @@ -34,9 +34,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) @@ -56,12 +58,55 @@ assert(result == expected); } +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, 1)"), std::tuple{1, CharT('1'), 1.0}); + + 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 --- a/libcxx/test/support/format.functions.common.h +++ b/libcxx/test/support/format.functions.common.h @@ -49,11 +49,17 @@ // 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 LWEG 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; @@ -86,6 +92,9 @@ 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'; @@ -124,7 +133,7 @@ } 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 @@ -134,6 +143,52 @@ } }; +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.