From 78002b8035a27606a9c55cd4e206aee76d0c30aa Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Mon, 9 Oct 2023 20:33:30 -0500 Subject: [PATCH 1/5] Validity check simple replacement fields A custom formatter for a user-defined type might reject omitted format-specs: ```c++ struct A {}; template<> struct std::formatter { constexpr auto parse(std::format_parse_context const& ctx) { std::string_view s("narf"); auto [i, j] = std::mismatch(ctx.begin(), ctx.end(), s.begin(), s.end()); if (j != s.end()) throw std::runtime_error("you didn't say the magic word!"); return i; } auto format(A, std::format_context& ctx) const { return ctx.out(); } }; int main() { std::ignore = std::format("{}", A()); } ``` Per [format.fmt.string]/3 this should be rejected, but MSVC instead accepts and throws at runtime. This implementation seems most likely to not cause problems with existing code; it matches the runtime behavior of _Default_arg_formatter on custom formatters (basic_format_arg<_Context>::handle), so it should accept any format() call that would succeed at runtime. --- stl/inc/format | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 190c29557a6..faf8f63c20c 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3517,7 +3517,10 @@ struct _Format_checker { consteval explicit _Format_checker(basic_string_view<_CharT> _Fmt) noexcept : _Parse_context(_Fmt, _Num_args), _Parse_funcs{&_Compile_time_parse_format_specs<_Args, _ParseContext>...} {} constexpr void _On_text(const _CharT*, const _CharT*) const noexcept {} - constexpr void _On_replacement_field(size_t, const _CharT*) const noexcept {} + constexpr void _On_replacement_field(size_t, const _CharT*) const { + _ParseContext _Parse_ctx({}); + _Parse_ctx.advance_to(_Parse_funcs[_Id](_Parse_ctx)); + } constexpr const _CharT* _On_format_specs(const size_t _Id, const _CharT* _First, const _CharT*) { _Parse_context.advance_to(_Parse_context.begin() + (_First - _Parse_context.begin()._Unwrapped())); if (_Id < _Num_args) { From 76b6f5b392e98c11cf0aa702ea1bfa265bfc8d8e Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Mon, 9 Oct 2023 22:04:32 -0500 Subject: [PATCH 2/5] Update stl/inc/format Co-authored-by: A. Jiang --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index faf8f63c20c..cfb2975af15 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3517,7 +3517,7 @@ struct _Format_checker { consteval explicit _Format_checker(basic_string_view<_CharT> _Fmt) noexcept : _Parse_context(_Fmt, _Num_args), _Parse_funcs{&_Compile_time_parse_format_specs<_Args, _ParseContext>...} {} constexpr void _On_text(const _CharT*, const _CharT*) const noexcept {} - constexpr void _On_replacement_field(size_t, const _CharT*) const { + constexpr void _On_replacement_field(size_t _Id, const _CharT*) const { _ParseContext _Parse_ctx({}); _Parse_ctx.advance_to(_Parse_funcs[_Id](_Parse_ctx)); } From b10844558861146d3ef8515008729c9c6db9180d Mon Sep 17 00:00:00 2001 From: Ed Catmur Date: Mon, 9 Oct 2023 22:05:04 -0500 Subject: [PATCH 3/5] Update format --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index cfb2975af15..d09dd41b210 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3517,7 +3517,7 @@ struct _Format_checker { consteval explicit _Format_checker(basic_string_view<_CharT> _Fmt) noexcept : _Parse_context(_Fmt, _Num_args), _Parse_funcs{&_Compile_time_parse_format_specs<_Args, _ParseContext>...} {} constexpr void _On_text(const _CharT*, const _CharT*) const noexcept {} - constexpr void _On_replacement_field(size_t _Id, const _CharT*) const { + constexpr void _On_replacement_field(const size_t _Id, const _CharT*) const { _ParseContext _Parse_ctx({}); _Parse_ctx.advance_to(_Parse_funcs[_Id](_Parse_ctx)); } From 1a3ec0da4037a36aaee23c6b87db894a348ada7b Mon Sep 17 00:00:00 2001 From: Edward Catmur Date: Fri, 13 Oct 2023 14:00:45 +0100 Subject: [PATCH 4/5] fix tests --- .../tests/P0645R10_text_formatting_custom_formatting/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e15a100d782..928ff8aa893 100644 --- a/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_custom_formatting/test.cpp @@ -57,7 +57,7 @@ struct not_const_formattable_type { template <> struct std::formatter { - basic_format_parse_context::iterator parse(basic_format_parse_context& parse_ctx) { + constexpr basic_format_parse_context::iterator parse(basic_format_parse_context& parse_ctx) { if (parse_ctx.begin() != parse_ctx.end()) { throw format_error{"only empty specs please"}; } @@ -71,7 +71,7 @@ struct std::formatter { template <> struct std::formatter { - basic_format_parse_context::iterator parse(basic_format_parse_context& parse_ctx) { + constexpr basic_format_parse_context::iterator parse(basic_format_parse_context& parse_ctx) { if (parse_ctx.begin() != parse_ctx.end()) { throw format_error{"only empty specs please"}; } From b33fd74455894057dad59beb24a8372a3ca9f8ec Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 25 Oct 2023 14:45:34 -0700 Subject: [PATCH 5/5] Casey's review comment --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 4a4a9b49c22..0ffd0c8ddba 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3521,7 +3521,7 @@ struct _Format_checker { constexpr void _On_text(const _CharT*, const _CharT*) const noexcept {} constexpr void _On_replacement_field(const size_t _Id, const _CharT*) const { _ParseContext _Parse_ctx({}); - _Parse_ctx.advance_to(_Parse_funcs[_Id](_Parse_ctx)); + (void) _Parse_funcs[_Id](_Parse_ctx); } constexpr const _CharT* _On_format_specs(const size_t _Id, const _CharT* _First, const _CharT*) { _Parse_context.advance_to(_Parse_context.begin() + (_First - _Parse_context.begin()._Unwrapped()));