From ec9bba0b7c6084fab75518ad00cfd9fb82ae2f56 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Thu, 1 Oct 2020 14:22:14 -0700 Subject: [PATCH 01/32] add parse_precision. --- stl/inc/format | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 83631ab8089..d79259599c5 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -220,7 +220,6 @@ constexpr const _CharT* _Parse_precision(const _CharT* _Begin, const _CharT* _En if (_Begin != _End) { _Ch = *_Begin; } - if ('0' <= _Ch && _Ch <= '9') { int _Precision = 0; _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); From 98222f2c5e71b0f99d0a4082e8d07d7b9cb6c458 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Mon, 12 Oct 2020 13:44:44 -0700 Subject: [PATCH 02/32] adopt style review comments. --- stl/inc/format | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/format b/stl/inc/format index d79259599c5..83631ab8089 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -220,6 +220,7 @@ constexpr const _CharT* _Parse_precision(const _CharT* _Begin, const _CharT* _En if (_Begin != _End) { _Ch = *_Begin; } + if ('0' <= _Ch && _Ch <= '9') { int _Precision = 0; _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); From 735b93da4df8b68005b90429a81c5675ba210587 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Tue, 13 Oct 2020 17:44:08 -0700 Subject: [PATCH 03/32] actually call the test functions. --- tests/std/tests/P0645R10_text_formatting_parsing/test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index 85da914a9ac..70586dc706a 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -287,6 +287,5 @@ int main() { test_parse_format_specs(); static_assert(test_parse_format_specs()); static_assert(test_parse_format_specs()); - return 0; } From 19e27afe90ca8e0596e2f7dcab1eaf38853b6f3b Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Thu, 1 Oct 2020 14:22:14 -0700 Subject: [PATCH 04/32] add parse_precision. --- stl/inc/format | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 83631ab8089..d79259599c5 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -220,7 +220,6 @@ constexpr const _CharT* _Parse_precision(const _CharT* _Begin, const _CharT* _En if (_Begin != _End) { _Ch = *_Begin; } - if ('0' <= _Ch && _Ch <= '9') { int _Precision = 0; _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); From fe59f7a0a910c2c0dc29f9c52ff2e114dd14d330 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Mon, 12 Oct 2020 13:44:44 -0700 Subject: [PATCH 05/32] adopt style review comments. --- stl/inc/format | 1 + 1 file changed, 1 insertion(+) diff --git a/stl/inc/format b/stl/inc/format index d79259599c5..83631ab8089 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -220,6 +220,7 @@ constexpr const _CharT* _Parse_precision(const _CharT* _Begin, const _CharT* _En if (_Begin != _End) { _Ch = *_Begin; } + if ('0' <= _Ch && _Ch <= '9') { int _Precision = 0; _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); From 272f691d41623d932a1a2c1a58c5b3be3d54d7a4 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Mon, 12 Oct 2020 15:47:16 -0700 Subject: [PATCH 06/32] add parse format spec --- .../tests/P0645R10_text_formatting_parsing/test.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index 70586dc706a..b2079e346d8 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -80,6 +80,18 @@ struct testing_callbacks { constexpr void _On_type(CharT type) { assert(type == expected_type); } + constexpr void _On_sign(_Sign sgn) { + assert(sgn = expected_sign); + } + constexpr void _On_hash() { + assert(expected_hash); + } + constexpr void _On_zero() { + assert(expected_zero); + } + constexpr void _On_type(CharT type) { + assert(type = expected_type); + } }; template testing_callbacks(_Align, basic_string_view) -> testing_callbacks; From 674345932e652aae39fdf1a374b0f56d143846c2 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Mon, 12 Oct 2020 16:28:18 -0700 Subject: [PATCH 07/32] add tests for parse_format_specs --- tests/std/tests/P0645R10_text_formatting_parsing/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index b2079e346d8..03541faae3e 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -81,7 +81,7 @@ struct testing_callbacks { assert(type == expected_type); } constexpr void _On_sign(_Sign sgn) { - assert(sgn = expected_sign); + assert(sgn == expected_sign); } constexpr void _On_hash() { assert(expected_hash); @@ -90,7 +90,7 @@ struct testing_callbacks { assert(expected_zero); } constexpr void _On_type(CharT type) { - assert(type = expected_type); + assert(type == expected_type); } }; template From 337e45ff843a3f701bbe4c3736a31962bfdfd464 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Tue, 13 Oct 2020 17:19:06 -0700 Subject: [PATCH 08/32] initial commit for string_parsing --- stl/inc/format | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/stl/inc/format b/stl/inc/format index 83631ab8089..2562a3866f4 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -761,6 +761,32 @@ public: } }; +template +auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { + return _STD visit(_STD forward<_Visitor>(_Vis), _Arg._Value); +} + +// The top level set of parsing "actions". +// This is like the _Parse_spec_callbacks concept but need not be a concept +// because it's not really involved in any user customization. +template +struct _Format_handler { + basic_format_parse_context<_CharT> _Parse_context; + _Context _Ctx; + + explicit _Format_hander(typename _ArgFormatter::iterator _Out, basic_string_view<_Char> _Str, + basic_format_args<_Context> _Format_args, locale& _Loc) + : _Parse_context(_Str), _Ctx(_Out, _Format_args, _Loc) {} + + void _On_text(const _CharT* _Begin, const _CharT* _End) { + /* TODO: output the text */ + } + + void _On_replacement_field(int _Id, const _CharT*) {} + + const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) {} +}; + using format_context = basic_format_context, string::value_type>; using wformat_context = basic_format_context, wstring::value_type>; using format_args = basic_format_args; From 7f82c69ad56648505e9d4919bcff286fc0fd96bf Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Tue, 20 Oct 2020 13:51:11 -0700 Subject: [PATCH 09/32] remove duplicate test frunctions --- .../tests/P0645R10_text_formatting_parsing/test.cpp | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index 03541faae3e..70586dc706a 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -80,18 +80,6 @@ struct testing_callbacks { constexpr void _On_type(CharT type) { assert(type == expected_type); } - constexpr void _On_sign(_Sign sgn) { - assert(sgn == expected_sign); - } - constexpr void _On_hash() { - assert(expected_hash); - } - constexpr void _On_zero() { - assert(expected_zero); - } - constexpr void _On_type(CharT type) { - assert(type == expected_type); - } }; template testing_callbacks(_Align, basic_string_view) -> testing_callbacks; From 306b65c40cfb83a4b5c62b313c47451a2b19f1d1 Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Tue, 20 Oct 2020 17:33:48 -0700 Subject: [PATCH 10/32] on_text and some on_format_specs --- stl/inc/format | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 2562a3866f4..5c4a6b7b3f2 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -774,17 +774,33 @@ struct _Format_handler { basic_format_parse_context<_CharT> _Parse_context; _Context _Ctx; - explicit _Format_hander(typename _ArgFormatter::iterator _Out, basic_string_view<_Char> _Str, + explicit _Format_handler(typename _ArgFormatter::iterator _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, locale& _Loc) : _Parse_context(_Str), _Ctx(_Out, _Format_args, _Loc) {} void _On_text(const _CharT* _Begin, const _CharT* _End) { - /* TODO: output the text */ + auto _Size = _End - _Begin; + auto _Out = _Ctx.out(); + _STD copy_n(_Begin, _Size, _Out); + _Ctx.advance_to(_Out); } - void _On_replacement_field(int _Id, const _CharT*) {} + int _On_arg_id() { + return _Parse_context.next_arg_id(); + } + + int _On_arg_id(int _Id) { + return _Parse_context.check_arg_id(_Id); + } - const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) {} + void _On_replacement_field(int _Id, const _CharT*) { + auto _Arg = _Ctx.arg(_Id); + } + + const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { + _Parse_context.advance_to(_Begin); + auto _Arg = _Ctx.arg(_Id); + } }; using format_context = basic_format_context, string::value_type>; From 3ceb7fe8255d41fe2a7f8e0ae3ffcb8ec1ec61bb Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Thu, 19 Nov 2020 13:41:08 -0800 Subject: [PATCH 11/32] add basic_format_specs and the formatter dispatcher. --- stl/inc/format | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/stl/inc/format b/stl/inc/format index 5c4a6b7b3f2..bd6c588a78b 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -302,6 +302,16 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* return _Begin; } +template +struct _Basic_format_specs { + int width, precision; + char type; + _Align _Alignment; + _Sign _Sgn; + bool _Alt; + _Char _Fill[4]; +} + template struct formatter; @@ -766,6 +776,27 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { return _STD visit(_STD forward<_Visitor>(_Vis), _Arg._Value); } +// Dispatcher to call enabled custom formatters, and do nothing otherwise. +template +class _Custom_formatter_dispatcher { +private: + using _Char_type = typename _Context::char_type; + + basic_format_parse_context<_Char_type>& _Parse_ctx; + _Context& _Ctx; + +public: + explicit constexpr _Custom_formatter_dispatcher(basic_format_parse_context<_Char_type>& _Parse_ctx, _Context& _Ctx) + : _Parse_ctx(_Parse_ctx), _Ctx(_Ctx) {} + + void operator()(typename basic_format_arg<_Context>::handle h) const { + h.format(_Parse_ctx, _Ctx); + } + + template + void operator()(_Ty) const {} +}; + // The top level set of parsing "actions". // This is like the _Parse_spec_callbacks concept but need not be a concept // because it's not really involved in any user customization. From 598bc3966367dcf3c62e16eba5b66d6058fba9ff Mon Sep 17 00:00:00 2001 From: Charles Barto Date: Thu, 19 Nov 2020 13:41:32 -0800 Subject: [PATCH 12/32] forgot a semicolon. --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index bd6c588a78b..7ee0684e5ed 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -310,7 +310,7 @@ struct _Basic_format_specs { _Sign _Sgn; bool _Alt; _Char _Fill[4]; -} +}; template struct formatter; From 167dc6fe36b073d2cc15d8d25d1a36cbf21176ed Mon Sep 17 00:00:00 2001 From: Charlie Barto Date: Fri, 11 Dec 2020 15:44:48 -0800 Subject: [PATCH 13/32] now we need to do _Write... :| --- stl/inc/format | 125 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 123 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 7ee0684e5ed..4c3ab348e04 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -30,6 +30,14 @@ _STL_DISABLE_CLANG_WARNINGS _STD_BEGIN +template +struct _Overloaded : _Tys... { + using _Tys::operator()...; +}; + +template +_Overloaded(_Tys...) -> _Overloaded<_Tys...>; + class format_error : public runtime_error { using runtime_error::runtime_error; }; @@ -38,6 +46,29 @@ enum class _Align { _None, _Left, _Right, _Center }; enum class _Sign { _None, _Plus, _Minus, _Space }; +enum class _Type { + _None, + _Int32, + _Int64, + _UInt32, + _UInt64, + _Bool, + _Char, + _Last_integer_type = _Char, + _Float32, + _Float64, + _Last_numeric_type = _Float64, + _CString, + _String, + _Pointer, + _Custom +}; +constexpr bool _Is_integral_fmt_type(_Type _Ty) { + return _Ty > _Type::_None && _Ty <= _Type::_Last_integer_type; +} +constexpr bool _Is_numeric_fmt_type(_Type _Ty) { + return _Ty > _Type::_None && _Ty <= _Type::_Last_numeric_type; +} struct _Auto_id_tag {}; // clang-format off @@ -304,14 +335,93 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* template struct _Basic_format_specs { - int width, precision; - char type; + int _Width, _Precision; + char _Type; _Align _Alignment; _Sign _Sgn; bool _Alt; _Char _Fill[4]; }; +// Extremely dull set of parsing callbacks that just fills a struct with parsed data. +template +class _Specs_setter { + explicit constexpr _Specs_setter(_Basic_format_specs<_CharT> _Specs) : _Specs(_Specs) {} + + constexpr void _On_align(_Align _Aln) { + _Specs._Alignment = _Aln; + } + + constexpr void _On_fill(basic_string_view<_CharT> _Sv) { + _Specs._Fill = _Sv; + } + + constexpr void _On_sign(_Sign _Sgn) { + _Specs._Sgn = _Sgn; + } + + constexpr void _On_hash() { + _Specs._Alt = true; + } + + constexpr void _On_zero() { + _Specs._Alignment = _Align::_None; + _Specs._Fill[0] = '0'; + } + + constexpr void _On_width(int _Width) { + _Specs._Width = _Width; + } + + constexpr void _On_Precision(int _Precision) { + _Specs._Precision = _Precision; + } + + constexpr void _On_type(_CharT _Type) { + _Specs._Type = static_cast<_CharT>(_Type); + } + +protected: + _Basic_format_specs<_CharT> _Specs; +}; + + +class _Numeric_specs_checker { + _Type _Arg_type = _Type::_None; + +public: + constexpr _Numeric_specs_checker(_Type _Arg_type) : _Arg_type(_Arg_type) {} + + constexpr void _Require_numeric_argument() { + if (!_Is_numeric_fmt_type(_Arg_type)) { + throw format_error("Format specifier requires numeric argument."); + } + } + + constexpr void _Check_sign() { + _Require_numeric_argument(); + if (_Is_integral_fmt_type(_Arg_type) && _Arg_type != _Type::_Int32 && _Arg_type != _Type::_Int64 + && _Arg_type != _Type::_Char) { + throw format_error("Format specifier requires signed argument."); + } + } + + constexpr void _Check_precision() { + if (_Is_integral_fmt_type(_Arg_type) || _Arg_type == _Type::_Pointer) { + throw format_error("Precision not allowed for this argument type."); + } + } +}; + +class _Specs_checker { + _Numeric_specs_checker _Numeric_checker; + +public: + constexpr _Specs_checker(_Type _Arg_type) : _Numeric_checker(_Arg_type) {} + + constexpr void _On_align(_Align _Aln) {} +}; + template struct formatter; @@ -776,6 +886,12 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { return _STD visit(_STD forward<_Visitor>(_Vis), _Arg._Value); } +template +_OutputIt _Write(_OutputIt _Out, monostate) { + _STL_INTERNAL_CHECK(false); + return _Out; +} + // Dispatcher to call enabled custom formatters, and do nothing otherwise. template class _Custom_formatter_dispatcher { @@ -826,6 +942,11 @@ struct _Format_handler { void _On_replacement_field(int _Id, const _CharT*) { auto _Arg = _Ctx.arg(_Id); + _Ctx.advance_to(visit_format_arg( + _Overloaded { + + } + ) } const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { From e8bb72f0992a1663e8a5307e3e4edda44017cff2 Mon Sep 17 00:00:00 2001 From: Charles Date: Thu, 21 Jan 2021 17:30:22 -0800 Subject: [PATCH 14/32] more work on format checkers --- stl/inc/format | 154 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 141 insertions(+), 13 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 4c3ab348e04..84a05cee93b 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -333,16 +333,17 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* return _Begin; } -template +template struct _Basic_format_specs { int _Width, _Precision; char _Type; _Align _Alignment; _Sign _Sgn; bool _Alt; - _Char _Fill[4]; + _CharT _Fill[4]; }; + // Extremely dull set of parsing callbacks that just fills a struct with parsed data. template class _Specs_setter { @@ -386,6 +387,59 @@ protected: }; +template +constexpr int _Get_dynamic_specs(_FormatArg _Arg) { + unsigned long long _Val = visit_format_arg(_Handler(), _Arg); + if (_Val > static_cast((_STD numeric_limits::max)())) { + throw format_error("Number is too big."); + } + return static_cast(value); +} + +template +constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, int _Arg_id) { + // note: while this is parameterized on the _Arg_id type in libfmt we don't + // need to do that in std::format because it's only called with either an integer + // id or a named id (which we do not support in std::format) + auto _Arg = _Ctx.arg(_Arg_id); + if (!_Arg) { + throw format_error("Argument not found."); + } + return _Arg; +} + +template +class _Specs_handler : public _Specs_setter { +public: + using _CharT = typename _Context::char_type; + + constexpr _Specs_handler(_Basic_format_specs<_CharT>& _Specs, _ParseContext& _Parse_ctx, _Context& _Ctx) + : _Specs_setter<_CharT>(_Specs), _Parse_ctx(_Parse_ctx), _Ctx(_Ctx) {} + + template + constexpr void _On_dynamic_width(_Id _Arg_id) { + this->_Specs._Width = _Get_dynamic_specs<_Width_checker>(_Get_arg(_Arg_id)); + } + + template + constexpr void _On_dynamic_precision(_Id _Arg_id) { + this->_Specs._Precision = _Get_dynamic_specs<_Precision_checker>(_Get_arg(_Arg_id)); + } + +private: + _ParseContext& _Parse_ctx; + _Context& _Ctx; + + constexpr basic_format_arg<_Context> _Get_arg(_Auto_id_tag) { + return _STD _Get_arg(_Ctx, _Parse_ctx.next_arg_id()); + } + + constexpr basic_format_arg<_Context> _Get_arg(int _Arg_id) { + _Parse_ctx.check_arg_id(_Arg_id); + return _STD _Get_arg(_Ctx, _Arg_id); + } +}; + class _Numeric_specs_checker { _Type _Arg_type = _Type::_None; @@ -413,13 +467,66 @@ public: } }; -class _Specs_checker { +class _Width_checker { +public: + template + constexpr unsigned long long operator()(_Ty _Value) { + if constexpr (is_integral_v<_Ty>) { + if constexpr (is_signed_v<_Ty>) { + if (_Value < 0) { + throw format_error("Negative width."); + } + } + return static_cast(_Value); + } + throw format_error("Width is not an integer."); + } +}; + +class _Precision_checker { +public: + template + constexpr unsigned long long operator()(_Ty _Value) { + if constexpr (is_integral_v<_Ty>) { + if constexpr (is_signed_v<_Ty>) { + if (_Value < 0) { + throw format_error("Nagative precision."); + } + } + return static_cast +class _Specs_checker : public _Handler { _Numeric_specs_checker _Numeric_checker; public: - constexpr _Specs_checker(_Type _Arg_type) : _Numeric_checker(_Arg_type) {} + constexpr _Specs_checker(const _Handler& _Handler_inst, _Type _Arg_type) + : _Handler(_Handler_inst), _Numeric_checker(_Arg_type) {} + + // _On_align has no checking, since we don't implement numeric alignments. + constexpr void _On_sign(_Sign _Sgn) { + _Numeric_checker._Check_sign(); + _Handler::_On_sign(_Sgn); + } + + constexpr void _On_hash() { + _Numeric_checker._Require_numeric_argument(); + _Handler::_On_hash(); + } - constexpr void _On_align(_Align _Aln) {} + constexpr void _On_zero() { + _Numeric_checker._Require_numeric_argument(); + _Handler::_On_zero(); + } + + constexpr void _On_precision(int _Precision) { + _Numeric_checker._Check_precision(); + _Handler::_On_precision(_Precision); + } }; template @@ -913,16 +1020,40 @@ public: void operator()(_Ty) const {} }; +// this is the visitor that's used for replacement fields, +// it could be a generic lambda (with overloaded), but that's +// bad for throughput +template +struct _Default_arg_formatter { + using _Context = basic_format_context<_OutputIt, _CharT>; + + _OutputIt _Out; + basic_format_args<_Context> _Args; + locale& _Loc; + + template + _OutputIt operator()(_Ty _Val) { + return _Write<_CharT>(_Out, _Val); + } + + _OutputIt operator()(typename basic_format_arg<_Context>::handle _Handle) { + basic_format_parse_context<_CharT> _Parse_ctx({}); + basic_format_context<_OutputIt, _CHarT> _Format_ctx(_Out, _Args, _Loc); + _Handle.format(parse_ctx, format_ctx); + return _Format_ctx.out(); + } +}; + // The top level set of parsing "actions". // This is like the _Parse_spec_callbacks concept but need not be a concept // because it's not really involved in any user customization. -template +template struct _Format_handler { basic_format_parse_context<_CharT> _Parse_context; _Context _Ctx; - explicit _Format_handler(typename _ArgFormatter::iterator _Out, basic_string_view<_CharT> _Str, - basic_format_args<_Context> _Format_args, locale& _Loc) + explicit _Format_handler( + typename _OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, locale& _Loc) : _Parse_context(_Str), _Ctx(_Out, _Format_args, _Loc) {} void _On_text(const _CharT* _Begin, const _CharT* _End) { @@ -942,11 +1073,8 @@ struct _Format_handler { void _On_replacement_field(int _Id, const _CharT*) { auto _Arg = _Ctx.arg(_Id); - _Ctx.advance_to(visit_format_arg( - _Overloaded { - - } - ) + _Ctx.advance_to( + visit_format_arg(_Default_arg_formatter<_OutputIt, _CharT>{_Ctx.out(), _Ctx.args(), _Ctx.locale()}, _Arg)); } const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { From 06fa2fe3233c5743e8c369c58b8513f4898518c4 Mon Sep 17 00:00:00 2001 From: Charles Date: Thu, 21 Jan 2021 18:56:48 -0800 Subject: [PATCH 15/32] add _Format_arg_type_to_enum --- stl/inc/format | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/stl/inc/format b/stl/inc/format index 84a05cee93b..b3bf2271ba0 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -882,6 +882,9 @@ public: size_t _Arg_index = 0; (_Store(_Arg_index++, _Vals), ...); } + constexpr _Type _Get_type() { + return _STD visit(_Format_arg_type_to_enum<_CharT>{}, _Value); + } }; template @@ -1080,6 +1083,9 @@ struct _Format_handler { const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { _Parse_context.advance_to(_Begin); auto _Arg = _Ctx.arg(_Id); + basic_format_specs<_CharT> _Specs; + _Specs_checker<_Specs_handler, _Context>> _Handler( + _Specs_handler, _Context>(_Specs, _Parse_context, _Ctx), _Arg.type()) } }; From 755e7bd30d68ae666d545848713dce0a6824415d Mon Sep 17 00:00:00 2001 From: Charles Date: Fri, 22 Jan 2021 16:41:19 -0800 Subject: [PATCH 16/32] add char* writer --- stl/inc/format | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index b3bf2271ba0..329a527d533 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1002,6 +1002,16 @@ _OutputIt _Write(_OutputIt _Out, monostate) { return _Out; } +template +_OutputIt _Write(_OutputIt _Out, const _CharT* _Value) { + if (!_Value) { + throw format_error("String pointer is null."); + } + while (*_Value) { + *_Out++ = *_Value++; + } +} + // Dispatcher to call enabled custom formatters, and do nothing otherwise. template class _Custom_formatter_dispatcher { @@ -1081,11 +1091,17 @@ struct _Format_handler { } const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { - _Parse_context.advance_to(_Begin); + _Parse_context.advance_to(_Parse_ctx.begin() + (_Begin - &*_Parse_ctx.begin())); auto _Arg = _Ctx.arg(_Id); basic_format_specs<_CharT> _Specs; _Specs_checker<_Specs_handler, _Context>> _Handler( - _Specs_handler, _Context>(_Specs, _Parse_context, _Ctx), _Arg.type()) + _Specs_handler, _Context>(_Specs, _Parse_context, _Ctx), + _Arg._Get_type()); + _Begin = _Parse_format_specs(_Begin, _End, _Handler); + if (_Begin == _End || *_Begin != '}') { + throw format_error("Missing '}' in format string."); + } + _Ctx.advance_to(visit_format_arg()) } }; From a6830dd7e18635ede77f90694e613adfccee4d89 Mon Sep 17 00:00:00 2001 From: Charles Date: Fri, 22 Jan 2021 17:45:30 -0800 Subject: [PATCH 17/32] make it compile --- stl/inc/format | 209 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 170 insertions(+), 39 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 329a527d533..f3d75cdd2e3 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -333,6 +333,137 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* return _Begin; } +// TODO: test coverage +template +class basic_format_parse_context { +public: + using char_type = _CharT; + using const_iterator = typename basic_string_view<_CharT>::const_iterator; + using iterator = const_iterator; + +private: + basic_string_view<_CharT> _Format_string; + size_t _Num_args; + // The standard says this is size_t, however we use ptrdiff_t to save some space + // by not having to store the indexing mode. Below is a more detailed explanation + // of how this works. + ptrdiff_t _Next_arg_id = 0; + +public: + constexpr explicit basic_format_parse_context(basic_string_view<_CharT> _Fmt, size_t _Num_args_ = 0) noexcept + : _Format_string(_Fmt), _Num_args(_Num_args_) {} + basic_format_parse_context(const basic_format_parse_context&) = delete; + basic_format_parse_context& operator=(const basic_format_parse_context&) = delete; + + _NODISCARD constexpr const_iterator begin() const noexcept { + return _Format_string.begin(); + } + _NODISCARD constexpr const_iterator end() const noexcept { + return _Format_string.end(); + } + constexpr void advance_to(const_iterator _It) { + _Format_string.remove_prefix(_It - begin()); + } + + // While the standard presents an exposition only enum value for + // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. + // _Next_arg_id == 0 means unknown + // _Next_arg_id > 0 means automatic + // _Next_arg_id == -1 means manual + constexpr size_t next_arg_id() { + if (_Next_arg_id >= 0) { + return _Next_arg_id++; + } + throw format_error("Can not switch from manual to automatic indexing"); + } + constexpr void check_arg_id(size_t _Id) { + (void) _Id; + if (_Next_arg_id > 0) { + throw format_error("Can not switch from automatic to manual indexing"); + } + _Next_arg_id = -1; + } +}; + +template +class basic_format_arg { +public: + class handle; + +private: + using _CharT = typename _Context::char_type; + + using _Format_arg_value = variant, handle>; + + _Format_arg_value _Value; + + template + struct _Format_arg_type_to_enum { + constexpr _Type operator()(_STD monostate) { + return _Type::_None; + } + constexpr _Type operator()(int32_t) { + return _Type::_Int32; + } + constexpr _Type operator()(int64_t) { + return _Type::_Int64; + } + constexpr _Type operator()(uint32_t) { + return _Type::_UInt32; + } + constexpr _Type operator()(uint64_t) { + return _Type::_UInt64; + } + constexpr _Type operator()(bool) { + return _Type::_Bool; + } + constexpr _Type operator()(_CharT) { + return _Type::_Char; + } + constexpr _Type operator()(float) { + return _Type::_Float32; + } + constexpr _Type operator()(double) { + return _Type::_Float64; + } + constexpr _Type operator()(const _CharT*) { + return _Type::_CString; + } + constexpr _Type operator()(const basic_string_view<_CharT>&) { + return _Type::_String; + } + constexpr _Type operator()(const void*) { + return _Type::_Pointer; + } + constexpr _Type operator()(const basic_format_arg::handle&) { + return _Type::_Custom; + } + }; + +public: + class handle { + private: + const void* _Ptr; + void (*_Format)(basic_format_parse_context<_CharT>& _Parse_ctx, _Context _Format_ctx, const void*); + friend basic_format_arg; + + public: + void format(basic_format_parse_context<_CharT>& _Parse_ctx, _Context& _Format_ctx) { + _Format(_Parse_ctx, _Format_ctx, _Ptr); + } + }; + + basic_format_arg() noexcept = default; + explicit operator bool() const noexcept { + return !_STD holds_alternative(_Value); + } + constexpr _Type _Get_type() { + return _STD visit(_Format_arg_type_to_enum<_CharT>{}, _Value); + } +}; + template struct _Basic_format_specs { int _Width, _Precision; @@ -393,7 +524,7 @@ constexpr int _Get_dynamic_specs(_FormatArg _Arg) { if (_Val > static_cast((_STD numeric_limits::max)())) { throw format_error("Number is too big."); } - return static_cast(value); + return static_cast(_Val); } template @@ -408,6 +539,38 @@ constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, int _Arg_id) { return _Arg; } +class _Width_checker { +public: + template + constexpr unsigned long long operator()(_Ty _Value) { + if constexpr (is_integral_v<_Ty>) { + if constexpr (is_signed_v<_Ty>) { + if (_Value < 0) { + throw format_error("Negative width."); + } + } + return static_cast(_Value); + } + throw format_error("Width is not an integer."); + } +}; + +class _Precision_checker { +public: + template + constexpr unsigned long long operator()(_Ty _Value) { + if constexpr (is_integral_v<_Ty>) { + if constexpr (is_signed_v<_Ty>) { + if (_Value < 0) { + throw format_error("Nagative precision."); + } + } + return static_cast(_Value); + } + throw format_error("Precision is not an integer."); + } +}; + template class _Specs_handler : public _Specs_setter { public: @@ -467,38 +630,6 @@ public: } }; -class _Width_checker { -public: - template - constexpr unsigned long long operator()(_Ty _Value) { - if constexpr (is_integral_v<_Ty>) { - if constexpr (is_signed_v<_Ty>) { - if (_Value < 0) { - throw format_error("Negative width."); - } - } - return static_cast(_Value); - } - throw format_error("Width is not an integer."); - } -}; - -class _Precision_checker { -public: - template - constexpr unsigned long long operator()(_Ty _Value) { - if constexpr (is_integral_v<_Ty>) { - if constexpr (is_signed_v<_Ty>) { - if (_Value < 0) { - throw format_error("Nagative precision."); - } - } - return static_cast class _Specs_checker : public _Handler { _Numeric_specs_checker _Numeric_checker; @@ -1051,8 +1182,8 @@ struct _Default_arg_formatter { _OutputIt operator()(typename basic_format_arg<_Context>::handle _Handle) { basic_format_parse_context<_CharT> _Parse_ctx({}); - basic_format_context<_OutputIt, _CHarT> _Format_ctx(_Out, _Args, _Loc); - _Handle.format(parse_ctx, format_ctx); + basic_format_context<_OutputIt, _CharT> _Format_ctx(_Out, _Args, _Loc); + _Handle.format(_Parse_ctx, _Format_ctx); return _Format_ctx.out(); } }; @@ -1066,7 +1197,7 @@ struct _Format_handler { _Context _Ctx; explicit _Format_handler( - typename _OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, locale& _Loc) + _OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, locale& _Loc) : _Parse_context(_Str), _Ctx(_Out, _Format_args, _Loc) {} void _On_text(const _CharT* _Begin, const _CharT* _End) { @@ -1091,9 +1222,9 @@ struct _Format_handler { } const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { - _Parse_context.advance_to(_Parse_ctx.begin() + (_Begin - &*_Parse_ctx.begin())); + _Parse_context.advance_to(_Parse_context.begin() + (_Begin - &*_Parse_context.begin())); auto _Arg = _Ctx.arg(_Id); - basic_format_specs<_CharT> _Specs; + _Basic_format_specs<_CharT> _Specs; _Specs_checker<_Specs_handler, _Context>> _Handler( _Specs_handler, _Context>(_Specs, _Parse_context, _Ctx), _Arg._Get_type()); @@ -1101,7 +1232,7 @@ struct _Format_handler { if (_Begin == _End || *_Begin != '}') { throw format_error("Missing '}' in format string."); } - _Ctx.advance_to(visit_format_arg()) + _STL_INTERNAL_CHECK(false); } }; From d26d4b362bd6e6c1db3bed21bfa2fff625fbfaa9 Mon Sep 17 00:00:00 2001 From: Charles Date: Fri, 29 Jan 2021 17:57:54 -0800 Subject: [PATCH 18/32] flesh out format_args_store. --- stl/inc/format | 206 +++++++++++++----- .../env.lst | 4 + .../test.cpp | 10 + 3 files changed, 165 insertions(+), 55 deletions(-) create mode 100644 tests/std/tests/P0645R10_text_formatting_formatting/env.lst create mode 100644 tests/std/tests/P0645R10_text_formatting_formatting/test.cpp diff --git a/stl/inc/format b/stl/inc/format index f3d75cdd2e3..f0aa4ef46b9 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -12,6 +12,7 @@ #pragma message("The contents of are available only with C++20 concepts support.") #else // ^^^ !defined(__cpp_lib_concepts) / defined(__cpp_lib_concepts) vvv +#include #include #include #include @@ -71,6 +72,57 @@ constexpr bool _Is_numeric_fmt_type(_Type _Ty) { } struct _Auto_id_tag {}; +template +class basic_format_parse_context { +public: + using char_type = _CharT; + using const_iterator = typename basic_string_view<_CharT>::const_iterator; + using iterator = const_iterator; + +private: + basic_string_view<_CharT> _Format_string; + size_t _Num_args; + // The standard says this is size_t, however we use ptrdiff_t to save some space + // by not having to store the indexing mode. Below is a more detailed explanation + // of how this works. + ptrdiff_t _Next_arg_id = 0; + +public: + constexpr explicit basic_format_parse_context(basic_string_view<_CharT> _Fmt, size_t _Num_args_ = 0) noexcept + : _Format_string(_Fmt), _Num_args(_Num_args_) {} + basic_format_parse_context(const basic_format_parse_context&) = delete; + basic_format_parse_context& operator=(const basic_format_parse_context&) = delete; + + _NODISCARD constexpr const_iterator begin() const noexcept { + return _Format_string.begin(); + } + _NODISCARD constexpr const_iterator end() const noexcept { + return _Format_string.end(); + } + constexpr void advance_to(const_iterator _It) { + _Format_string.remove_prefix(_It - begin()); + } + + // While the standard presents an exposition only enum value for + // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. + // _Next_arg_id == 0 means unknown + // _Next_arg_id > 0 means automatic + // _Next_arg_id == -1 means manual + constexpr size_t next_arg_id() { + if (_Next_arg_id >= 0) { + return _Next_arg_id++; + } + throw format_error("Can not switch from manual to automatic indexing"); + } + constexpr void check_arg_id(size_t _Id) { + (void) _Id; + if (_Next_arg_id > 0) { + throw format_error("Can not switch from automatic to manual indexing"); + } + _Next_arg_id = -1; + } +}; + // clang-format off template concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { @@ -92,6 +144,15 @@ concept _Parse_arg_id_callbacks = requires(_Ty _At) { { _At._On_auto_id() } -> same_as; { _At._On_manual_id(_STD declval()) } -> same_as; }; + +template +concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { + { _At._Parse_context } -> _STD convertible_to&>; + { _At._On_text(_Begin, _End) } -> same_as; + { _At._On_replacement_field(int{}, _STD declval()) } -> same_as; + { _At._On_format_specs(int{}, _Begin, _End) } -> same_as; +}; + // clang-format on // we need to implement this ourselves because from_chars does not work with wide characters @@ -224,6 +285,19 @@ struct _Precision_adapter { } }; +template +struct _Id_adapter { + basic_format_parse_context<_CharT>& _Parse_context; + int _Arg_id = 0; + constexpr void _On_auto_id() { + _Arg_id = _Parse_context.next_arg_id(); + } + constexpr void _On_manual_id(int _Id) { + _Parse_context.check_arg_id(_Id); + _Arg_id = _Id; + } +}; + template _Callbacks_type> constexpr const _CharT* _Parse_width(const _CharT* _Begin, const _CharT* _End, _Callbacks_type&& _Callbacks) { _STL_INTERNAL_CHECK(_Begin != _End); @@ -333,57 +407,79 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* return _Begin; } -// TODO: test coverage -template -class basic_format_parse_context { -public: - using char_type = _CharT; - using const_iterator = typename basic_string_view<_CharT>::const_iterator; - using iterator = const_iterator; - -private: - basic_string_view<_CharT> _Format_string; - size_t _Num_args; - // The standard says this is size_t, however we use ptrdiff_t to save some space - // by not having to store the indexing mode. Below is a more detailed explanation - // of how this works. - ptrdiff_t _Next_arg_id = 0; - -public: - constexpr explicit basic_format_parse_context(basic_string_view<_CharT> _Fmt, size_t _Num_args_ = 0) noexcept - : _Format_string(_Fmt), _Num_args(_Num_args_) {} - basic_format_parse_context(const basic_format_parse_context&) = delete; - basic_format_parse_context& operator=(const basic_format_parse_context&) = delete; - - _NODISCARD constexpr const_iterator begin() const noexcept { - return _Format_string.begin(); - } - _NODISCARD constexpr const_iterator end() const noexcept { - return _Format_string.end(); - } - constexpr void advance_to(const_iterator _It) { - _Format_string.remove_prefix(_It - begin()); +template _HandlerT> +constexpr const _CharT* _Parse_replacement_field(const _CharT* _Begin, const _CharT* _End, _HandlerT&& _Handler) { + ++_Begin; + if (_Begin == _End) { + throw format_error("Invalid format string."); } - // While the standard presents an exposition only enum value for - // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. - // _Next_arg_id == 0 means unknown - // _Next_arg_id > 0 means automatic - // _Next_arg_id == -1 means manual - constexpr size_t next_arg_id() { - if (_Next_arg_id >= 0) { - return _Next_arg_id++; + if (*_Begin == '}') { + // string was "{}", and we have a replacement field + _Handler._On_replacement_field(_Handler._On_auto_id()); + } else if (*_Begin == '{') { + // string was "{{", so we have a literal "{" to print + _Handler._On_text(_Begin, _Begin + 1); + } else { + _Id_adapter<_CharT> _Adapter{_Handler._Parse_context}; + _Begin = _Parse_arg_id(_Begin, _End, _Adapter); + _CharT _Ch = _CharT{}; + if (_Begin != _End) { + _Ch = *_Begin; + } + if (_Ch == '}') { + _Handler._On_replacement_field(_Adapter._Arg_id, _Begin); + } else if (_Ch == ':') { + _Begin = _Handler._On_format_specs(_Adapter._Arg_id, _Begin + 1, _End); + if (_Begin == _End || *_Begin != '}') { + throw format_error("Unknown format specifier."); + } + } else { + throw format_error("Missing '}' in format string."); } - throw format_error("Can not switch from manual to automatic indexing"); } - constexpr void check_arg_id(size_t _Id) { - (void) _Id; - if (_Next_arg_id > 0) { - throw format_error("Can not switch from automatic to manual indexing"); + return _Begin + 1; +} + +template _HandlerT> +constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _HandlerT&& _Handler) { + auto _Begin = _Format_str.data(); + auto _End = _Begin + _Format_str.size(); + struct _Writer { + constexpr void operator()(const _CharT* _Begin, const _CharT* _End) { + if (_Begin == _End) { + return; + } + for (;;) { + const _CharT* _Pt = _STD find(_Begin, _End, '}'); + if (_Pt == _End) { + return _Handler._On_text(_Begin, _End); + } + ++_Pt; + if (_Pt == _End || *_Pt != '}') { + throw format_error("Unmatched '}' in format string."); + } + _Handler._On_text(_Begin, _Pt); + _Begin = _Pt + 1; + } } - _Next_arg_id = -1; + _HandlerT& _Handler; + } _Writer_loop{_Handler}; + while (_Begin != _End) { + const _CharT* _Pt = _Begin; + if (*_Begin != '{') { + _Pt = _STD find(_Begin + 1, _End, '{', _Pt); + if (_Pt == _End) { + return _Writer_loop(_Begin, _End); + } + } + _Writer_loop(_Begin, _End); + _Begin = _Parse_replacement_field(_Pt, _End, _Handler); } -}; +} + +// TODO: test coverage + template class basic_format_arg { @@ -1189,8 +1285,6 @@ struct _Default_arg_formatter { }; // The top level set of parsing "actions". -// This is like the _Parse_spec_callbacks concept but need not be a concept -// because it's not really involved in any user customization. template struct _Format_handler { basic_format_parse_context<_CharT> _Parse_context; @@ -1207,14 +1301,6 @@ struct _Format_handler { _Ctx.advance_to(_Out); } - int _On_arg_id() { - return _Parse_context.next_arg_id(); - } - - int _On_arg_id(int _Id) { - return _Parse_context.check_arg_id(_Id); - } - void _On_replacement_field(int _Id, const _CharT*) { auto _Arg = _Ctx.arg(_Id); _Ctx.advance_to( @@ -1251,6 +1337,16 @@ auto make_wformat_args(const _Args&... _Vals) { return _Format_arg_store<_Context, _Args...>{_Vals...}; } +template +using format_args_t = basic_format_args>; + +template +_Out vformat_to( + _Out _OutputIt, const locale& _Loc, string_view _Fmt, format_args_t, char> _Args) { + _Format_handler<_Out, char, basic_format_context<_Out, char>> _Handler(_OutputIt, _Fmt, _Args, _Loc); + _Parse_format_string(_Fmt, _Handler); +} + // FUNCTION vformat string vformat(string_view _Fmt, format_args _Args); wstring vformat(wstring_view _Fmt, wformat_args _Args); diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/env.lst b/tests/std/tests/P0645R10_text_formatting_formatting/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_formatting/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\concepts_matrix.lst diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp new file mode 100644 index 00000000000..d7266c1534f --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include + +// TODO: fill in tests + +int main() {} From 7f18de4817be1045f44e3d43a7debece3d4f3a5d Mon Sep 17 00:00:00 2001 From: Charles Date: Fri, 29 Jan 2021 18:25:32 -0800 Subject: [PATCH 19/32] don't need overloaded --- stl/inc/format | 8 -------- 1 file changed, 8 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index f0aa4ef46b9..c648948f357 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -31,14 +31,6 @@ _STL_DISABLE_CLANG_WARNINGS _STD_BEGIN -template -struct _Overloaded : _Tys... { - using _Tys::operator()...; -}; - -template -_Overloaded(_Tys...) -> _Overloaded<_Tys...>; - class format_error : public runtime_error { using runtime_error::runtime_error; }; From dc29afbab664c1cce41b141ac8bea44101fbe3be Mon Sep 17 00:00:00 2001 From: Charles Date: Mon, 1 Feb 2021 13:29:04 -0800 Subject: [PATCH 20/32] get rid of a bunch of declvals. --- stl/inc/format | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index c648948f357..c10922edfbe 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -120,21 +120,21 @@ template concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { { _At._On_align(_Aln) } -> same_as; { _At._On_fill(_Sv) } -> same_as; - { _At._On_width(_STD declval()) } -> same_as; - { _At._On_dynamic_width(_STD declval()) } -> same_as; - { _At._On_dynamic_width(_STD declval<_Auto_id_tag>()) } -> same_as; - { _At._On_precision(_STD declval()) } -> same_as; - { _At._On_dynamic_precision(_STD declval()) } -> same_as; - { _At._On_dynamic_precision(_STD declval<_Auto_id_tag>()) } -> same_as; + { _At._On_width(int{}) } -> same_as; + { _At._On_dynamic_width(int{}) } -> same_as; + { _At._On_dynamic_width(_Auto_id_tag{}) } -> same_as; + { _At._On_precision(int{}) } -> same_as; + { _At._On_dynamic_precision(int{}) } -> same_as; + { _At._On_dynamic_precision(_Auto_id_tag{}) } -> same_as; { _At._On_sign(_Sgn) } -> same_as; { _At._On_hash() } -> same_as; { _At._On_zero() } -> same_as; - { _At._On_type(_STD declval<_CharT>()) } -> same_as; + { _At._On_type(_CharT{}) } -> same_as; }; template concept _Parse_arg_id_callbacks = requires(_Ty _At) { { _At._On_auto_id() } -> same_as; - { _At._On_manual_id(_STD declval()) } -> same_as; + { _At._On_manual_id(int{}) } -> same_as; }; template @@ -1310,6 +1310,7 @@ struct _Format_handler { if (_Begin == _End || *_Begin != '}') { throw format_error("Missing '}' in format string."); } + // TODO: implement format spec dispatching. _STL_INTERNAL_CHECK(false); } }; From ac5e58ae29c9211eb60c36638fa54afaf127665a Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 2 Feb 2021 17:49:38 -0800 Subject: [PATCH 21/32] compile vformat (does not link) --- stl/inc/format | 80 +++++++++++++------ .../test.cpp | 4 + .../P0645R10_text_formatting_parsing/test.cpp | 44 ++++++++++ 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index c10922edfbe..a1b1efdfa98 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -12,6 +12,7 @@ #pragma message("The contents of are available only with C++20 concepts support.") #else // ^^^ !defined(__cpp_lib_concepts) / defined(__cpp_lib_concepts) vvv +#include #include #include #include @@ -92,7 +93,9 @@ public: return _Format_string.end(); } constexpr void advance_to(const_iterator _It) { - _Format_string.remove_prefix(_It - begin()); + _STL_INTERNAL_CHECK(_It - begin() >= 0); + using _Size_type = typename basic_string_view<_CharT>::size_type; + _Format_string.remove_prefix(static_cast<_Size_type>(_It - begin())); } // While the standard presents an exposition only enum value for @@ -102,7 +105,7 @@ public: // _Next_arg_id == -1 means manual constexpr size_t next_arg_id() { if (_Next_arg_id >= 0) { - return _Next_arg_id++; + return static_cast(_Next_arg_id++); } throw format_error("Can not switch from manual to automatic indexing"); } @@ -134,15 +137,15 @@ concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, template concept _Parse_arg_id_callbacks = requires(_Ty _At) { { _At._On_auto_id() } -> same_as; - { _At._On_manual_id(int{}) } -> same_as; + { _At._On_manual_id(size_t{}) } -> same_as; }; template concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { - { _At._Parse_context } -> _STD convertible_to&>; + //{ _At._Parse_context } -> same_as>; { _At._On_text(_Begin, _End) } -> same_as; - { _At._On_replacement_field(int{}, _STD declval()) } -> same_as; - { _At._On_format_specs(int{}, _Begin, _End) } -> same_as; + { _At._On_replacement_field(size_t{}, _STD declval()) } -> same_as; + { _At._On_format_specs(size_t{}, _Begin, _End) } -> same_as; }; // clang-format on @@ -197,7 +200,7 @@ constexpr const _CharT* _Parse_arg_id(const _CharT* _Begin, const _CharT* _End, if (_Begin == _End || (*_Begin != '}' && *_Begin != ':')) { throw format_error("Invalid format string."); } - _Callbacks._On_manual_id(_Index); + _Callbacks._On_manual_id(static_cast(_Index)); return _Begin; } // This is where we would parse named arg ids if std::format were to support them. @@ -256,7 +259,7 @@ struct _Width_adapter { constexpr void _On_auto_id() { _Callbacks._On_dynamic_width(_Auto_id_tag{}); } - constexpr void _On_manual_id(int _Id) { + constexpr void _On_manual_id(size_t _Id) { _Callbacks._On_dynamic_width(_Id); } }; @@ -272,7 +275,7 @@ struct _Precision_adapter { constexpr void _On_auto_id() { _Callbacks._On_dynamic_precision(_Auto_id_tag{}); } - constexpr void _On_manual_id(int _Id) { + constexpr void _On_manual_id(size_t _Id) { _Callbacks._On_dynamic_precision(_Id); } }; @@ -280,11 +283,11 @@ struct _Precision_adapter { template struct _Id_adapter { basic_format_parse_context<_CharT>& _Parse_context; - int _Arg_id = 0; + size_t _Arg_id = 0; constexpr void _On_auto_id() { _Arg_id = _Parse_context.next_arg_id(); } - constexpr void _On_manual_id(int _Id) { + constexpr void _On_manual_id(size_t _Id) { _Parse_context.check_arg_id(_Id); _Arg_id = _Id; } @@ -408,7 +411,7 @@ constexpr const _CharT* _Parse_replacement_field(const _CharT* _Begin, const _Ch if (*_Begin == '}') { // string was "{}", and we have a replacement field - _Handler._On_replacement_field(_Handler._On_auto_id()); + _Handler._On_replacement_field(_Handler._Parse_context.next_arg_id(), _Begin); } else if (*_Begin == '{') { // string was "{{", so we have a literal "{" to print _Handler._On_text(_Begin, _Begin + 1); @@ -460,7 +463,7 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand while (_Begin != _End) { const _CharT* _Pt = _Begin; if (*_Begin != '{') { - _Pt = _STD find(_Begin + 1, _End, '{', _Pt); + _Pt = _STD find(_Begin + 1, _End, '{'); if (_Pt == _End) { return _Writer_loop(_Begin, _End); } @@ -479,6 +482,9 @@ public: class handle; private: + template + friend auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Ctx> _Arg) -> decltype(_Vis(0)); + using _CharT = typename _Context::char_type; using _Format_arg_value = variant class _Specs_setter { +public: explicit constexpr _Specs_setter(_Basic_format_specs<_CharT> _Specs) : _Specs(_Specs) {} constexpr void _On_align(_Align _Aln) { @@ -573,7 +583,11 @@ class _Specs_setter { } constexpr void _On_fill(basic_string_view<_CharT> _Sv) { - _Specs._Fill = _Sv; + if (_Sv.size() > 4) { + throw format_error("Invalid fill."); + } + _STD fill(_Specs._Fill, _Specs._Fill + 4, '\0'); + _STD copy(_Sv.begin(), _Sv.end(), _Specs._Fill); } constexpr void _On_sign(_Sign _Sgn) { @@ -593,7 +607,7 @@ class _Specs_setter { _Specs._Width = _Width; } - constexpr void _On_Precision(int _Precision) { + constexpr void _On_precision(int _Precision) { _Specs._Precision = _Precision; } @@ -616,7 +630,7 @@ constexpr int _Get_dynamic_specs(_FormatArg _Arg) { } template -constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, int _Arg_id) { +constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, size_t _Arg_id) { // note: while this is parameterized on the _Arg_id type in libfmt we don't // need to do that in std::format because it's only called with either an integer // id or a named id (which we do not support in std::format) @@ -685,7 +699,7 @@ private: return _STD _Get_arg(_Ctx, _Parse_ctx.next_arg_id()); } - constexpr basic_format_arg<_Context> _Get_arg(int _Arg_id) { + constexpr basic_format_arg<_Context> _Get_arg(size_t _Arg_id) { _Parse_ctx.check_arg_id(_Arg_id); return _STD _Get_arg(_Ctx, _Arg_id); } @@ -1194,6 +1208,10 @@ public: template using formatter_type = formatter<_Ty, _CharT>; + constexpr basic_format_context( + _Out _OutputIt, basic_format_args _Ctx_args, const locale& _Loc) + : _OutputIt(_OutputIt), _Args(_Ctx_args), _Loc(_Loc) {} + basic_format_arg arg(size_t _Id) const { return _Args.get(_Id); } @@ -1208,10 +1226,14 @@ public: // TODO: IDL support probably required _OutputIt = _It; } + + const basic_format_args& _Get_args() const { + return _Args; + } }; template -auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { +auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)) { return _STD visit(_STD forward<_Visitor>(_Vis), _Arg._Value); } @@ -1229,6 +1251,14 @@ _OutputIt _Write(_OutputIt _Out, const _CharT* _Value) { while (*_Value) { *_Out++ = *_Value++; } + return _Out; +} + +template +_OutputIt _Write(_OutputIt _Out, _Ty _Val) { + (void) _Val; + _STL_INTERNAL_CHECK(false); + return _Out; } // Dispatcher to call enabled custom formatters, and do nothing otherwise. @@ -1261,7 +1291,7 @@ struct _Default_arg_formatter { _OutputIt _Out; basic_format_args<_Context> _Args; - locale& _Loc; + locale _Loc; template _OutputIt operator()(_Ty _Val) { @@ -1283,7 +1313,7 @@ struct _Format_handler { _Context _Ctx; explicit _Format_handler( - _OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, locale& _Loc) + _OutputIt _Out, basic_string_view<_CharT> _Str, basic_format_args<_Context> _Format_args, const locale& _Loc) : _Parse_context(_Str), _Ctx(_Out, _Format_args, _Loc) {} void _On_text(const _CharT* _Begin, const _CharT* _End) { @@ -1293,13 +1323,13 @@ struct _Format_handler { _Ctx.advance_to(_Out); } - void _On_replacement_field(int _Id, const _CharT*) { + void _On_replacement_field(size_t _Id, const _CharT*) { auto _Arg = _Ctx.arg(_Id); - _Ctx.advance_to( - visit_format_arg(_Default_arg_formatter<_OutputIt, _CharT>{_Ctx.out(), _Ctx.args(), _Ctx.locale()}, _Arg)); + _Ctx.advance_to(visit_format_arg( + _Default_arg_formatter<_OutputIt, _CharT>{_Ctx.out(), _Ctx._Get_args(), _Ctx.locale()}, _Arg)); } - const _CharT* _On_format_specs(int _Id, const _CharT* _Begin, const _CharT* _End) { + const _CharT* _On_format_specs(size_t _Id, const _CharT* _Begin, const _CharT* _End) { _Parse_context.advance_to(_Parse_context.begin() + (_Begin - &*_Parse_context.begin())); auto _Arg = _Ctx.arg(_Id); _Basic_format_specs<_CharT> _Specs; @@ -1312,6 +1342,7 @@ struct _Format_handler { } // TODO: implement format spec dispatching. _STL_INTERNAL_CHECK(false); + return _Begin; } }; @@ -1338,6 +1369,7 @@ _Out vformat_to( _Out _OutputIt, const locale& _Loc, string_view _Fmt, format_args_t, char> _Args) { _Format_handler<_Out, char, basic_format_context<_Out, char>> _Handler(_OutputIt, _Fmt, _Args, _Loc); _Parse_format_string(_Fmt, _Handler); + return _Handler._Ctx.out(); } // FUNCTION vformat diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index d7266c1534f..7e28bdf95fb 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -3,8 +3,12 @@ #include #include +#include #include // TODO: fill in tests +template std::back_insert_iterator std::vformat_to(std::back_insert_iterator, const locale&, + std::string_view, std::format_args_t, char>); + int main() {} diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index 70586dc706a..f38e233df09 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -30,6 +30,22 @@ struct choose_literal { #define TYPED_LITERAL(CharT, Literal) (choose_literal::choose(Literal, L##Literal)) +template +struct noop_testing_callbacks { + constexpr void _On_align(_Align) {} + constexpr void _On_fill(basic_string_view) {} + constexpr void _On_width(int) {} + constexpr void _On_dynamic_width(int) {} + constexpr void _On_dynamic_width(_Auto_id_tag) {} + constexpr void _On_precision(int) {} + constexpr void _On_dynamic_precision(int) {} + constexpr void _On_dynamic_precision(_Auto_id_tag) {} + constexpr void _On_sign(_Sign) {} + constexpr void _On_hash() {} + constexpr void _On_zero() {} + constexpr void _On_type(CharT) {} +}; + template struct testing_callbacks { _Align expected_alignment = _Align::_None; @@ -262,6 +278,23 @@ constexpr bool test_parse_format_specs() { return true; } +template +constexpr bool test_specs_setter() { + // just instantiate for now. + _Basic_format_specs specs = {}; + _Specs_setter setter(specs); + + (void) setter; + return true; +} + +template +constexpr bool test_specs_checker() { + _Specs_checker> checker(noop_testing_callbacks{}, _Type::_Float32); + (void) checker; + return true; +} + int main() { test_parse_align(); test_parse_align(); @@ -287,5 +320,16 @@ int main() { test_parse_format_specs(); static_assert(test_parse_format_specs()); static_assert(test_parse_format_specs()); + + test_specs_setter(); + test_specs_setter(); + static_assert(test_specs_setter()); + static_assert(test_specs_setter()); + + test_specs_checker(); + test_specs_checker(); + static_assert(test_specs_checker()); + static_assert(test_specs_checker()); + return 0; } From ca4a9664cc6aba79ebd50b586524037a4ace8c5a Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 2 Feb 2021 19:12:36 -0800 Subject: [PATCH 22/32] make width and precision unsigned int internally (instead of int) --- stl/inc/format | 39 +++++++++---------- .../P0645R10_text_formatting_parsing/test.cpp | 26 ++++++------- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index a1b1efdfa98..63ed7b3b24f 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -123,11 +123,11 @@ template concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { { _At._On_align(_Aln) } -> same_as; { _At._On_fill(_Sv) } -> same_as; - { _At._On_width(int{}) } -> same_as; - { _At._On_dynamic_width(int{}) } -> same_as; + { _At._On_width(unsigned{}) } -> same_as; + { _At._On_dynamic_width(size_t{}) } -> same_as; { _At._On_dynamic_width(_Auto_id_tag{}) } -> same_as; - { _At._On_precision(int{}) } -> same_as; - { _At._On_dynamic_precision(int{}) } -> same_as; + { _At._On_precision(unsigned{}) } -> same_as; + { _At._On_dynamic_precision(size_t{}) } -> same_as; { _At._On_dynamic_precision(_Auto_id_tag{}) } -> same_as; { _At._On_sign(_Sgn) } -> same_as; { _At._On_hash() } -> same_as; @@ -152,9 +152,9 @@ concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Be // we need to implement this ourselves because from_chars does not work with wide characters template -constexpr const _CharT* _Parse_nonnegative_integer(const _CharT* _Begin, const _CharT* _End, int& _Integer) { +constexpr const _CharT* _Parse_nonnegative_integer(const _CharT* _Begin, const _CharT* _End, unsigned int& _Value) { _STL_INTERNAL_CHECK(_Begin != _End && '0' <= *_Begin && *_Begin <= '9'); - unsigned int _Value = 0; + _Value = 0; constexpr unsigned int _Max_int = static_cast((numeric_limits::max)()); constexpr unsigned int _Big_int = _Max_int / 10; @@ -169,7 +169,6 @@ constexpr const _CharT* _Parse_nonnegative_integer(const _CharT* _Begin, const _ if (_Value > _Max_int) { throw format_error("Number is too big"); } - _Integer = static_cast(_Value); return _Begin; } @@ -184,7 +183,7 @@ constexpr const _CharT* _Parse_arg_id(const _CharT* _Begin, const _CharT* _End, } if (_Ch >= '0' && _Ch <= '9') { - int _Index = 0; + unsigned int _Index = 0; // arg_id is not allowed to have any leading zeros, but is allowed to be // equal to zero (but not '00'). So if _Ch is zero we skip the parsing, leave // _Index set to zero and let the validity checks below ensure that the arg_id @@ -200,7 +199,7 @@ constexpr const _CharT* _Parse_arg_id(const _CharT* _Begin, const _CharT* _End, if (_Begin == _End || (*_Begin != '}' && *_Begin != ':')) { throw format_error("Invalid format string."); } - _Callbacks._On_manual_id(static_cast(_Index)); + _Callbacks._On_manual_id(_Index); return _Begin; } // This is where we would parse named arg ids if std::format were to support them. @@ -297,8 +296,8 @@ template _Callbacks_type> constexpr const _CharT* _Parse_width(const _CharT* _Begin, const _CharT* _End, _Callbacks_type&& _Callbacks) { _STL_INTERNAL_CHECK(_Begin != _End); if ('1' <= *_Begin && *_Begin <= '9') { - int _Value = 0; - _Begin = _Parse_nonnegative_integer(_Begin, _End, _Value); + unsigned int _Value = 0; + _Begin = _Parse_nonnegative_integer(_Begin, _End, _Value); _Callbacks._On_width(_Value); } else if (*_Begin == '{') { ++_Begin; @@ -322,8 +321,8 @@ constexpr const _CharT* _Parse_precision(const _CharT* _Begin, const _CharT* _En } if ('0' <= _Ch && _Ch <= '9') { - int _Precision = 0; - _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); + unsigned int _Precision = 0; + _Begin = _Parse_nonnegative_integer(_Begin, _End, _Precision); _Callbacks._On_precision(_Precision); } else if (_Ch == '{') { ++_Begin; @@ -563,7 +562,7 @@ public: template struct _Basic_format_specs { - int _Width, _Precision; + unsigned int _Width, _Precision; char _Type; _Align _Alignment; _Sign _Sgn; @@ -603,11 +602,11 @@ public: _Specs._Fill[0] = '0'; } - constexpr void _On_width(int _Width) { + constexpr void _On_width(unsigned int _Width) { _Specs._Width = _Width; } - constexpr void _On_precision(int _Precision) { + constexpr void _On_precision(unsigned int _Precision) { _Specs._Precision = _Precision; } @@ -621,12 +620,12 @@ protected: template -constexpr int _Get_dynamic_specs(_FormatArg _Arg) { +constexpr unsigned int _Get_dynamic_specs(_FormatArg _Arg) { unsigned long long _Val = visit_format_arg(_Handler(), _Arg); - if (_Val > static_cast((_STD numeric_limits::max)())) { + if (_Val > (_STD numeric_limits::max)()) { throw format_error("Number is too big."); } - return static_cast(_Val); + return static_cast(_Val); } template @@ -756,7 +755,7 @@ public: _Handler::_On_zero(); } - constexpr void _On_precision(int _Precision) { + constexpr void _On_precision(unsigned int _Precision) { _Numeric_checker._Check_precision(); _Handler::_On_precision(_Precision); } diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index f38e233df09..a3d2937dedb 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -34,11 +34,11 @@ template struct noop_testing_callbacks { constexpr void _On_align(_Align) {} constexpr void _On_fill(basic_string_view) {} - constexpr void _On_width(int) {} - constexpr void _On_dynamic_width(int) {} + constexpr void _On_width(unsigned int) {} + constexpr void _On_dynamic_width(size_t) {} constexpr void _On_dynamic_width(_Auto_id_tag) {} - constexpr void _On_precision(int) {} - constexpr void _On_dynamic_precision(int) {} + constexpr void _On_precision(unsigned int) {} + constexpr void _On_dynamic_precision(size_t) {} constexpr void _On_dynamic_precision(_Auto_id_tag) {} constexpr void _On_sign(_Sign) {} constexpr void _On_hash() {} @@ -51,11 +51,11 @@ struct testing_callbacks { _Align expected_alignment = _Align::_None; _Sign expected_sign = _Sign::_None; basic_string_view expected_fill; - int expected_width = -1; - int expected_dynamic_width = -1; + unsigned int expected_width = static_cast(-1); + size_t expected_dynamic_width = static_cast(-1); bool expected_auto_dynamic_width = false; - int expected_precision = -1; - int expected_dynamic_precision = -1; + unsigned int expected_precision = static_cast(-1); + size_t expected_dynamic_precision = static_cast(-1); bool expected_auto_dynamic_precision = false; bool expected_hash = false; bool expected_zero = false; @@ -66,19 +66,19 @@ struct testing_callbacks { constexpr void _On_fill(basic_string_view str_view) { assert(str_view == expected_fill); } - constexpr void _On_width(int width) { + constexpr void _On_width(unsigned int width) { assert(width == expected_width); } - constexpr void _On_dynamic_width(int id) { + constexpr void _On_dynamic_width(size_t id) { assert(id == expected_dynamic_width); } constexpr void _On_dynamic_width(_Auto_id_tag) { assert(expected_auto_dynamic_width); } - constexpr void _On_precision(int pre) { + constexpr void _On_precision(unsigned int pre) { assert(pre == expected_precision); } - constexpr void _On_dynamic_precision(int id) { + constexpr void _On_dynamic_precision(size_t id) { assert(id == expected_dynamic_precision); } constexpr void _On_dynamic_precision(_Auto_id_tag) { @@ -102,7 +102,7 @@ testing_callbacks(_Align, basic_string_view) -> testing_callbacks; struct testing_arg_id_callbacks { constexpr void _On_auto_id() {} - constexpr void _On_manual_id(int) {} + constexpr void _On_manual_id(size_t) {} }; template From a346bde9a9948d9ab8f5a87d95ab568b3d91d228 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 2 Feb 2021 20:03:22 -0800 Subject: [PATCH 23/32] add some comments for shat the various helpery classes actually do. --- stl/inc/format | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 63ed7b3b24f..c0e8776b2d4 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -571,7 +571,7 @@ struct _Basic_format_specs { }; -// Extremely dull set of parsing callbacks that just fills a struct with parsed data. +// _Parse_specs_callbacks that fill a _Basic_format_specs struct with the parsed data template class _Specs_setter { public: @@ -618,7 +618,9 @@ protected: _Basic_format_specs<_CharT> _Specs; }; - +// Actually fetch the value of an argument associated with a dynamic +// width or precision specifier. This will be called with either +// _Width_checker or _Precision_checker as "_Handler". template constexpr unsigned int _Get_dynamic_specs(_FormatArg _Arg) { unsigned long long _Val = visit_format_arg(_Handler(), _Arg); @@ -640,6 +642,8 @@ constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, size_t _Arg_id) { return _Arg; } +// Checks that the type and value of an argument associated with a dynamic +// width specifier is valid. class _Width_checker { public: template @@ -656,6 +660,8 @@ public: } }; +// Checks that the type and value of an argument associated with a dynamic +// precision specifier is valid. class _Precision_checker { public: template @@ -672,6 +678,10 @@ public: } }; +// Parses standard format specs into a _Basic_format_specs using _Specs_setter, and, +// in addition handles dynamic width and precision. This is seperate from _Specs setter +// because it needs to know about the current basic_format_parse_context and basic_format_context +// in order to fetch the width from the arguments. template class _Specs_handler : public _Specs_setter { public: @@ -703,7 +713,6 @@ private: return _STD _Get_arg(_Ctx, _Arg_id); } }; - class _Numeric_specs_checker { _Type _Arg_type = _Type::_None; @@ -730,7 +739,10 @@ public: } } }; - +// Uses _Numeric_specs_checker to check that the type of the argument printed by +// a replacement field with format specs actually satisfies the requirements for +// that format spec. If the requirements are met then calls the base class +// handler method. template class _Specs_checker : public _Handler { _Numeric_specs_checker _Numeric_checker; @@ -1281,9 +1293,10 @@ public: void operator()(_Ty) const {} }; -// this is the visitor that's used for replacement fields, +// This is the visitor that's used for "simple" replacement fields, // it could be a generic lambda (with overloaded), but that's -// bad for throughput +// bad for throughput. A simple replacement field is a replacement field +// that's just "{}", without any format specs. template struct _Default_arg_formatter { using _Context = basic_format_context<_OutputIt, _CharT>; From 932a1e658c0c64ca99789a03973bd6193a89c0e5 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 2 Feb 2021 20:06:27 -0800 Subject: [PATCH 24/32] formatting (heh) fixes. --- stl/inc/format | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index c0e8776b2d4..609534c3334 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -65,6 +65,7 @@ constexpr bool _Is_numeric_fmt_type(_Type _Ty) { } struct _Auto_id_tag {}; +// TODO: test coverage template class basic_format_parse_context { public: @@ -472,9 +473,8 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand } } -// TODO: test coverage - +// TODO: test coverage template class basic_format_arg { public: @@ -739,6 +739,7 @@ public: } } }; + // Uses _Numeric_specs_checker to check that the type of the argument printed by // a replacement field with format specs actually satisfies the requirements for // that format spec. If the requirements are met then calls the base class From ec2f7a266f665e8da6c1e944af49b75e3e181339 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 2 Feb 2021 20:36:43 -0800 Subject: [PATCH 25/32] uncomment and fix requirement for _Parse_replacement_field_callbacks --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 609534c3334..54543f867ec 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -143,7 +143,7 @@ concept _Parse_arg_id_callbacks = requires(_Ty _At) { template concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { - //{ _At._Parse_context } -> same_as>; + { _At._Parse_context }; { _At._On_text(_Begin, _End) } -> same_as; { _At._On_replacement_field(size_t{}, _STD declval()) } -> same_as; { _At._On_format_specs(size_t{}, _Begin, _End) } -> same_as; From c9229621c0e5d517b533256cabd2ae6707ab6791 Mon Sep 17 00:00:00 2001 From: Charles Date: Wed, 3 Feb 2021 10:03:49 -0800 Subject: [PATCH 26/32] don't write out the entire format string when encountering a replacement field. --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 54543f867ec..89fae3f13d9 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -468,7 +468,7 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand return _Writer_loop(_Begin, _End); } } - _Writer_loop(_Begin, _End); + _Writer_loop(_Begin, _Pt); _Begin = _Parse_replacement_field(_Pt, _End, _Handler); } } From 0347fa64043809b0d31018746912166e8fff573e Mon Sep 17 00:00:00 2001 From: Charles Date: Wed, 10 Feb 2021 11:43:19 -0800 Subject: [PATCH 27/32] resolve some code review comments --- stl/inc/format | 24 +++++++++++++++--------- tests/std/test.lst | 1 + 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 89fae3f13d9..da78672c084 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -12,7 +12,6 @@ #pragma message("The contents of are available only with C++20 concepts support.") #else // ^^^ !defined(__cpp_lib_concepts) / defined(__cpp_lib_concepts) vvv -#include #include #include #include @@ -22,6 +21,7 @@ #include #include #include +#include #pragma pack(push, _CRT_PACKING) #pragma warning(push, _STL_WARNING_LEVEL) @@ -60,7 +60,7 @@ enum class _Type { constexpr bool _Is_integral_fmt_type(_Type _Ty) { return _Ty > _Type::_None && _Ty <= _Type::_Last_integer_type; } -constexpr bool _Is_numeric_fmt_type(_Type _Ty) { +constexpr bool _Is_arithmetic_fmt_type(_Type _Ty) { return _Ty > _Type::_None && _Ty <= _Type::_Last_numeric_type; } struct _Auto_id_tag {}; @@ -280,16 +280,22 @@ struct _Precision_adapter { } }; +// _Parse_arg_id expects a handler when it finds an argument id, however +// _Parse_replacement_field actually needs to know the value of that argument ID to pass on +// to _Handler._On_replacement_field or _Handler._On_format_specs. This _Parse_arg_id wrapper +// stores the value of the arg id for later use, so _Parse_replacement_field has access to it. template struct _Id_adapter { basic_format_parse_context<_CharT>& _Parse_context; - size_t _Arg_id = 0; + size_t _Arg_id = static_cast(-1); constexpr void _On_auto_id() { _Arg_id = _Parse_context.next_arg_id(); + _STL_INTERNAL_CHECK(_Arg_id != static_cast(-1)); } constexpr void _On_manual_id(size_t _Id) { _Parse_context.check_arg_id(_Id); _Arg_id = _Id; + _STL_INTERNAL_CHECK(_Arg_id != static_cast(-1)); } }; @@ -446,7 +452,7 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand return; } for (;;) { - const _CharT* _Pt = _STD find(_Begin, _End, '}'); + const _CharT* _Pt = _STD _Find_unchecked(_Begin, _End, '}'); if (_Pt == _End) { return _Handler._On_text(_Begin, _End); } @@ -494,7 +500,7 @@ private: template struct _Format_arg_type_to_enum { - constexpr _Type operator()(_STD monostate) { + constexpr _Type operator()(monostate) { return _Type::_None; } constexpr _Type operator()(int32_t) { @@ -560,7 +566,7 @@ public: } }; -template +template struct _Basic_format_specs { unsigned int _Width, _Precision; char _Type; @@ -571,7 +577,7 @@ struct _Basic_format_specs { }; -// _Parse_specs_callbacks that fill a _Basic_format_specs struct with the parsed data +// Model of _Parse_specs_callbacks that fills a _Basic_format_specs with the parsed data. template class _Specs_setter { public: @@ -720,7 +726,7 @@ public: constexpr _Numeric_specs_checker(_Type _Arg_type) : _Arg_type(_Arg_type) {} constexpr void _Require_numeric_argument() { - if (!_Is_numeric_fmt_type(_Arg_type)) { + if (!_Is_arithmetic_fmt_type(_Arg_type)) { throw format_error("Format specifier requires numeric argument."); } } @@ -1320,7 +1326,7 @@ struct _Default_arg_formatter { }; // The top level set of parsing "actions". -template +template struct _Format_handler { basic_format_parse_context<_CharT> _Parse_context; _Context _Ctx; diff --git a/tests/std/test.lst b/tests/std/test.lst index e179bf9671e..9b6d214e47e 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -257,6 +257,7 @@ tests\P0645R10_text_formatting_args tests\P0645R10_text_formatting_death tests\P0645R10_text_formatting_parse_contexts tests\P0645R10_text_formatting_parsing +tests\P0645R10_text_formatting_formatting tests\P0660R10_jthread_and_cv_any tests\P0660R10_stop_token tests\P0660R10_stop_token_death From ed38d3831421b82c09ac7e01055ae37eb241e2aa Mon Sep 17 00:00:00 2001 From: Charlie Date: Tue, 16 Feb 2021 14:32:06 -0800 Subject: [PATCH 28/32] fix merge artifacts, convert from _Type to _Basic_format_arg_type --- stl/inc/format | 496 ++++++------------ .../P0645R10_text_formatting_parsing/test.cpp | 3 +- 2 files changed, 168 insertions(+), 331 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index da78672c084..41b34a57f0d 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -40,32 +40,69 @@ enum class _Align { _None, _Left, _Right, _Center }; enum class _Sign { _None, _Plus, _Minus, _Space }; -enum class _Type { +enum class _Basic_format_arg_type : uint8_t { _None, - _Int32, - _Int64, - _UInt32, - _UInt64, - _Bool, - _Char, - _Last_integer_type = _Char, - _Float32, - _Float64, - _Last_numeric_type = _Float64, - _CString, - _String, - _Pointer, - _Custom + _Int_type, + _UInt_type, + _Long_long_type, + _ULong_long_type, + _Bool_type, + _Char_type, + _Float_type, + _Double_type, + _Long_double_type, + _Pointer_type, + _CString_type, + _String_type, + _Custom_type, }; -constexpr bool _Is_integral_fmt_type(_Type _Ty) { - return _Ty > _Type::_None && _Ty <= _Type::_Last_integer_type; +static_assert(static_cast(_Basic_format_arg_type::_Custom_type) <= 16); + +constexpr bool _Is_integral_fmt_type(_Basic_format_arg_type _Ty) { + return _Ty > _Basic_format_arg_type::_None && _Ty <= _Basic_format_arg_type::_ULong_long_type; } -constexpr bool _Is_arithmetic_fmt_type(_Type _Ty) { - return _Ty > _Type::_None && _Ty <= _Type::_Last_numeric_type; +constexpr bool _Is_arithmetic_fmt_type(_Basic_format_arg_type _Ty) { + return _Ty > _Basic_format_arg_type::_None && _Ty <= _Basic_format_arg_type::_Long_double_type; } struct _Auto_id_tag {}; -// TODO: test coverage +// clang-format off +template +concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { + { _At._On_align(_Aln) } -> same_as; + { _At._On_fill(_Sv) } -> same_as; + { _At._On_width(unsigned{}) } -> same_as; + { _At._On_dynamic_width(size_t{}) } -> same_as; + { _At._On_dynamic_width(_Auto_id_tag{}) } -> same_as; + { _At._On_precision(unsigned{}) } -> same_as; + { _At._On_dynamic_precision(size_t{}) } -> same_as; + { _At._On_dynamic_precision(_Auto_id_tag{}) } -> same_as; + { _At._On_sign(_Sgn) } -> same_as; + { _At._On_hash() } -> same_as; + { _At._On_zero() } -> same_as; + { _At._On_type(_CharT{}) } -> same_as; +}; +template +concept _Parse_arg_id_callbacks = requires(_Ty _At) { + { _At._On_auto_id() } -> same_as; + { _At._On_manual_id(size_t{}) } -> same_as; +}; + +template +concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { + { _At._Parse_context }; + { _At._On_text(_Begin, _End) } -> same_as; + { _At._On_replacement_field(size_t{}, _STD declval()) } -> same_as; + { _At._On_format_specs(size_t{}, _Begin, _End) } -> same_as; +}; + +// clang-format on + +template +struct formatter; + +inline void _You_see_this_error_because_arg_id_is_out_of_range() noexcept {} + template class basic_format_parse_context { public: @@ -73,15 +110,6 @@ public: using const_iterator = typename basic_string_view<_CharT>::const_iterator; using iterator = const_iterator; -private: - basic_string_view<_CharT> _Format_string; - size_t _Num_args; - // The standard says this is size_t, however we use ptrdiff_t to save some space - // by not having to store the indexing mode. Below is a more detailed explanation - // of how this works. - ptrdiff_t _Next_arg_id = 0; - -public: constexpr explicit basic_format_parse_context(basic_string_view<_CharT> _Fmt, size_t _Num_args_ = 0) noexcept : _Format_string(_Fmt), _Num_args(_Num_args_) {} basic_format_parse_context(const basic_format_parse_context&) = delete; @@ -93,63 +121,124 @@ public: _NODISCARD constexpr const_iterator end() const noexcept { return _Format_string.end(); } - constexpr void advance_to(const_iterator _It) { - _STL_INTERNAL_CHECK(_It - begin() >= 0); - using _Size_type = typename basic_string_view<_CharT>::size_type; - _Format_string.remove_prefix(static_cast<_Size_type>(_It - begin())); + constexpr void advance_to(const const_iterator _It) { + _Adl_verify_range(_It, _Format_string.end()); + const auto _Diff = static_cast(_It._Unwrapped() - _Format_string._Unchecked_begin()); + _Format_string.remove_prefix(_Diff); } // While the standard presents an exposition only enum value for // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. - // _Next_arg_id == 0 means unknown // _Next_arg_id > 0 means automatic - // _Next_arg_id == -1 means manual + // _Next_arg_id == 0 means unknown + // _Next_arg_id < 0 means manual constexpr size_t next_arg_id() { - if (_Next_arg_id >= 0) { - return static_cast(_Next_arg_id++); + if (_Next_arg_id < 0) { + throw format_error("Can not switch from manual to automatic indexing"); } - throw format_error("Can not switch from manual to automatic indexing"); + + return static_cast(_Next_arg_id++); } - constexpr void check_arg_id(size_t _Id) { - (void) _Id; + + constexpr void check_arg_id(const size_t _Id) { + if (_STD is_constant_evaluated()) { + if (_Id >= _Num_args) { + _You_see_this_error_because_arg_id_is_out_of_range(); + } + } + if (_Next_arg_id > 0) { throw format_error("Can not switch from automatic to manual indexing"); } _Next_arg_id = -1; } -}; -// clang-format off -template -concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { - { _At._On_align(_Aln) } -> same_as; - { _At._On_fill(_Sv) } -> same_as; - { _At._On_width(unsigned{}) } -> same_as; - { _At._On_dynamic_width(size_t{}) } -> same_as; - { _At._On_dynamic_width(_Auto_id_tag{}) } -> same_as; - { _At._On_precision(unsigned{}) } -> same_as; - { _At._On_dynamic_precision(size_t{}) } -> same_as; - { _At._On_dynamic_precision(_Auto_id_tag{}) } -> same_as; - { _At._On_sign(_Sgn) } -> same_as; - { _At._On_hash() } -> same_as; - { _At._On_zero() } -> same_as; - { _At._On_type(_CharT{}) } -> same_as; -}; -template -concept _Parse_arg_id_callbacks = requires(_Ty _At) { - { _At._On_auto_id() } -> same_as; - { _At._On_manual_id(size_t{}) } -> same_as; +private: + basic_string_view<_CharT> _Format_string; + size_t _Num_args; + // The standard says this is size_t, however we use ptrdiff_t to save some space + // by not having to store the indexing mode. Above is a more detailed explanation + // of how this works. + ptrdiff_t _Next_arg_id = 0; }; -template -concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { - { _At._Parse_context }; - { _At._On_text(_Begin, _End) } -> same_as; - { _At._On_replacement_field(size_t{}, _STD declval()) } -> same_as; - { _At._On_format_specs(size_t{}, _Begin, _End) } -> same_as; -}; +template +class basic_format_arg { +public: + using _CharType = typename _Context::char_type; -// clang-format on + class handle { + private: + const void* _Ptr; + void (*_Format)(basic_format_parse_context<_CharType>& _Parse_ctx, _Context _Format_ctx, const void*); + friend basic_format_arg; + + public: + template + explicit handle(const _Ty& _Val) noexcept + : _Ptr(_STD addressof(_Val)), + _Format([](basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void* _Ptr) { + typename _Context::template formatter_type<_Ty> _Formatter; + _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); + _Format_ctx.advance_to(_Formatter.format(*static_cast(_Ptr), _Format_ctx)); + }) {} + + void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) { + _Format(_Parse_ctx, _Format_ctx, _Ptr); + } + }; + + // TRANSITION, LLVM-49072 + basic_format_arg() noexcept : _Active_state(_Basic_format_arg_type::_None), _No_state() {} + + explicit basic_format_arg(const int _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {} + explicit basic_format_arg(const unsigned int _Val) noexcept + : _Active_state(_Basic_format_arg_type::_UInt_type), _UInt_state(_Val) {} + explicit basic_format_arg(const long long _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Long_long_type), _Long_long_state(_Val) {} + explicit basic_format_arg(const unsigned long long _Val) noexcept + : _Active_state(_Basic_format_arg_type::_ULong_long_type), _ULong_long_state(_Val) {} + explicit basic_format_arg(const bool _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Bool_type), _Bool_state(_Val) {} + explicit basic_format_arg(const _CharType _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Char_type), _Char_state(_Val) {} + explicit basic_format_arg(const float _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Float_type), _Float_state(_Val) {} + explicit basic_format_arg(const double _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Double_type), _Double_state(_Val) {} + explicit basic_format_arg(const long double _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Long_double_type), _Long_double_state(_Val) {} + explicit basic_format_arg(const void* _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Pointer_type), _Pointer_state(_Val) {} + explicit basic_format_arg(const _CharType* _Val) noexcept + : _Active_state(_Basic_format_arg_type::_CString_type), _CString_state(_Val) {} + explicit basic_format_arg(const basic_string_view<_CharType> _Val) noexcept + : _Active_state(_Basic_format_arg_type::_String_type), _String_state(_Val) {} + explicit basic_format_arg(const handle _Val) noexcept + : _Active_state(_Basic_format_arg_type::_Custom_type), _Custom_state(_Val) {} + explicit operator bool() const noexcept { + return _Active_state != _Basic_format_arg_type::_None; + } + + _Basic_format_arg_type _Active_state = _Basic_format_arg_type::_None; + union { + monostate _No_state = monostate{}; + int _Int_state; + unsigned int _UInt_state; + long long _Long_long_state; + unsigned long long _ULong_long_state; + bool _Bool_state; + _CharType _Char_state; + float _Float_state; + double _Double_state; + long double _Long_double_state; + const void* _Pointer_state; + const _CharType* _CString_state; + basic_string_view<_CharType> _String_state; + handle _Custom_state; + }; +}; // we need to implement this ourselves because from_chars does not work with wide characters template @@ -480,92 +569,6 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand } -// TODO: test coverage -template -class basic_format_arg { -public: - class handle; - -private: - template - friend auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Ctx> _Arg) -> decltype(_Vis(0)); - - using _CharT = typename _Context::char_type; - - using _Format_arg_value = variant, handle>; - - _Format_arg_value _Value; - - template - struct _Format_arg_type_to_enum { - constexpr _Type operator()(monostate) { - return _Type::_None; - } - constexpr _Type operator()(int32_t) { - return _Type::_Int32; - } - constexpr _Type operator()(int64_t) { - return _Type::_Int64; - } - constexpr _Type operator()(uint32_t) { - return _Type::_UInt32; - } - constexpr _Type operator()(uint64_t) { - return _Type::_UInt64; - } - constexpr _Type operator()(bool) { - return _Type::_Bool; - } - constexpr _Type operator()(_CharT) { - return _Type::_Char; - } - constexpr _Type operator()(float) { - return _Type::_Float32; - } - constexpr _Type operator()(double) { - return _Type::_Float64; - } - constexpr _Type operator()(long double) { - return _Type::_Float64; - } - constexpr _Type operator()(const _CharT*) { - return _Type::_CString; - } - constexpr _Type operator()(const basic_string_view<_CharT>&) { - return _Type::_String; - } - constexpr _Type operator()(const void*) { - return _Type::_Pointer; - } - constexpr _Type operator()(const basic_format_arg::handle&) { - return _Type::_Custom; - } - }; - -public: - class handle { - private: - const void* _Ptr; - void (*_Format)(basic_format_parse_context<_CharT>& _Parse_ctx, _Context _Format_ctx, const void*); - friend basic_format_arg; - - public: - void format(basic_format_parse_context<_CharT>& _Parse_ctx, _Context& _Format_ctx) { - _Format(_Parse_ctx, _Format_ctx, _Ptr); - } - }; - - basic_format_arg() noexcept = default; - explicit operator bool() const noexcept { - return !_STD holds_alternative(_Value); - } - constexpr _Type _Get_type() { - return _STD visit(_Format_arg_type_to_enum<_CharT>{}, _Value); - } -}; - template struct _Basic_format_specs { unsigned int _Width, _Precision; @@ -720,10 +723,10 @@ private: } }; class _Numeric_specs_checker { - _Type _Arg_type = _Type::_None; + _Basic_format_arg_type _Arg_type = _Basic_format_arg_type::_None; public: - constexpr _Numeric_specs_checker(_Type _Arg_type) : _Arg_type(_Arg_type) {} + constexpr _Numeric_specs_checker(_Basic_format_arg_type _Arg_type) : _Arg_type(_Arg_type) {} constexpr void _Require_numeric_argument() { if (!_Is_arithmetic_fmt_type(_Arg_type)) { @@ -733,14 +736,15 @@ public: constexpr void _Check_sign() { _Require_numeric_argument(); - if (_Is_integral_fmt_type(_Arg_type) && _Arg_type != _Type::_Int32 && _Arg_type != _Type::_Int64 - && _Arg_type != _Type::_Char) { + if (_Is_integral_fmt_type(_Arg_type) && _Arg_type != _Basic_format_arg_type::_Int_type + && _Arg_type != _Basic_format_arg_type::_Long_long_type + && _Arg_type != _Basic_format_arg_type::_Char_type) { throw format_error("Format specifier requires signed argument."); } } constexpr void _Check_precision() { - if (_Is_integral_fmt_type(_Arg_type) || _Arg_type == _Type::_Pointer) { + if (_Is_integral_fmt_type(_Arg_type) || _Arg_type == _Basic_format_arg_type::_Pointer_type) { throw format_error("Precision not allowed for this argument type."); } } @@ -755,7 +759,7 @@ class _Specs_checker : public _Handler { _Numeric_specs_checker _Numeric_checker; public: - constexpr _Specs_checker(const _Handler& _Handler_inst, _Type _Arg_type) + constexpr _Specs_checker(const _Handler& _Handler_inst, _Basic_format_arg_type _Arg_type) : _Handler(_Handler_inst), _Numeric_checker(_Arg_type) {} // _On_align has no checking, since we don't implement numeric alignments. @@ -780,166 +784,6 @@ public: } }; -template -struct formatter; - -inline void _You_see_this_error_because_arg_id_is_out_of_range() noexcept {} - -template -class basic_format_parse_context { -public: - using char_type = _CharT; - using const_iterator = typename basic_string_view<_CharT>::const_iterator; - using iterator = const_iterator; - - constexpr explicit basic_format_parse_context(basic_string_view<_CharT> _Fmt, size_t _Num_args_ = 0) noexcept - : _Format_string(_Fmt), _Num_args(_Num_args_) {} - basic_format_parse_context(const basic_format_parse_context&) = delete; - basic_format_parse_context& operator=(const basic_format_parse_context&) = delete; - - _NODISCARD constexpr const_iterator begin() const noexcept { - return _Format_string.begin(); - } - _NODISCARD constexpr const_iterator end() const noexcept { - return _Format_string.end(); - } - constexpr void advance_to(const const_iterator _It) { - _Adl_verify_range(_It, _Format_string.end()); - const auto _Diff = static_cast(_It._Unwrapped() - _Format_string._Unchecked_begin()); - _Format_string.remove_prefix(_Diff); - } - - // While the standard presents an exposition only enum value for - // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. - // _Next_arg_id > 0 means automatic - // _Next_arg_id == 0 means unknown - // _Next_arg_id < 0 means manual - constexpr size_t next_arg_id() { - if (_Next_arg_id < 0) { - throw format_error("Can not switch from manual to automatic indexing"); - } - - return static_cast(_Next_arg_id++); - } - - constexpr void check_arg_id(const size_t _Id) { - if (_STD is_constant_evaluated()) { - if (_Id >= _Num_args) { - _You_see_this_error_because_arg_id_is_out_of_range(); - } - } - - if (_Next_arg_id > 0) { - throw format_error("Can not switch from automatic to manual indexing"); - } - _Next_arg_id = -1; - } - -private: - basic_string_view<_CharT> _Format_string; - size_t _Num_args; - // The standard says this is size_t, however we use ptrdiff_t to save some space - // by not having to store the indexing mode. Above is a more detailed explanation - // of how this works. - ptrdiff_t _Next_arg_id = 0; -}; - -enum class _Basic_format_arg_type : uint8_t { - _None, - _Int_type, - _UInt_type, - _Long_long_type, - _ULong_long_type, - _Bool_type, - _Char_type, - _Float_type, - _Double_type, - _Long_double_type, - _Pointer_type, - _CString_type, - _String_type, - _Custom_type, -}; -static_assert(static_cast(_Basic_format_arg_type::_Custom_type) <= 16); - -template -class basic_format_arg { -public: - using _CharType = typename _Context::char_type; - - class handle { - private: - const void* _Ptr; - void (*_Format)(basic_format_parse_context<_CharType>& _Parse_ctx, _Context _Format_ctx, const void*); - friend basic_format_arg; - - public: - template - explicit handle(const _Ty& _Val) noexcept - : _Ptr(_STD addressof(_Val)), - _Format([](basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx, const void* _Ptr) { - typename _Context::template formatter_type<_Ty> _Formatter; - _Parse_ctx.advance_to(_Formatter.parse(_Parse_ctx)); - _Format_ctx.advance_to(_Formatter.format(*static_cast(_Ptr), _Format_ctx)); - }) {} - - void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) { - _Format(_Parse_ctx, _Format_ctx, _Ptr); - } - }; - - // TRANSITION, LLVM-49072 - basic_format_arg() noexcept : _Active_state(_Basic_format_arg_type::_None), _No_state() {} - - explicit basic_format_arg(const int _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Int_type), _Int_state(_Val) {} - explicit basic_format_arg(const unsigned int _Val) noexcept - : _Active_state(_Basic_format_arg_type::_UInt_type), _UInt_state(_Val) {} - explicit basic_format_arg(const long long _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Long_long_type), _Long_long_state(_Val) {} - explicit basic_format_arg(const unsigned long long _Val) noexcept - : _Active_state(_Basic_format_arg_type::_ULong_long_type), _ULong_long_state(_Val) {} - explicit basic_format_arg(const bool _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Bool_type), _Bool_state(_Val) {} - explicit basic_format_arg(const _CharType _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Char_type), _Char_state(_Val) {} - explicit basic_format_arg(const float _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Float_type), _Float_state(_Val) {} - explicit basic_format_arg(const double _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Double_type), _Double_state(_Val) {} - explicit basic_format_arg(const long double _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Long_double_type), _Long_double_state(_Val) {} - explicit basic_format_arg(const void* _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Pointer_type), _Pointer_state(_Val) {} - explicit basic_format_arg(const _CharType* _Val) noexcept - : _Active_state(_Basic_format_arg_type::_CString_type), _CString_state(_Val) {} - explicit basic_format_arg(const basic_string_view<_CharType> _Val) noexcept - : _Active_state(_Basic_format_arg_type::_String_type), _String_state(_Val) {} - explicit basic_format_arg(const handle _Val) noexcept - : _Active_state(_Basic_format_arg_type::_Custom_type), _Custom_state(_Val) {} - explicit operator bool() const noexcept { - return _Active_state != _Basic_format_arg_type::_None; - } - - _Basic_format_arg_type _Active_state = _Basic_format_arg_type::_None; - union { - monostate _No_state = monostate{}; - int _Int_state; - unsigned int _UInt_state; - long long _Long_long_state; - unsigned long long _ULong_long_state; - bool _Bool_state; - _CharType _Char_state; - float _Float_state; - double _Double_state; - long double _Long_double_state; - const void* _Pointer_state; - const _CharType* _CString_state; - basic_string_view<_CharType> _String_state; - handle _Custom_state; - }; -}; - template auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { switch (_Arg._Active_state) { @@ -1133,9 +977,6 @@ public: size_t _Arg_index = 0; (_Store(_Arg_index++, _Vals), ...); } - constexpr _Type _Get_type() { - return _STD visit(_Format_arg_type_to_enum<_CharT>{}, _Value); - } }; template @@ -1250,11 +1091,6 @@ public: } }; -template -auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)) { - return _STD visit(_STD forward<_Visitor>(_Vis), _Arg._Value); -} - template _OutputIt _Write(_OutputIt _Out, monostate) { _STL_INTERNAL_CHECK(false); @@ -1354,7 +1190,7 @@ struct _Format_handler { _Basic_format_specs<_CharT> _Specs; _Specs_checker<_Specs_handler, _Context>> _Handler( _Specs_handler, _Context>(_Specs, _Parse_context, _Ctx), - _Arg._Get_type()); + _Arg._Active_state); _Begin = _Parse_format_specs(_Begin, _End, _Handler); if (_Begin == _End || *_Begin != '}') { throw format_error("Missing '}' in format string."); diff --git a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp index a3d2937dedb..9340bf3f3fa 100644 --- a/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parsing/test.cpp @@ -290,7 +290,8 @@ constexpr bool test_specs_setter() { template constexpr bool test_specs_checker() { - _Specs_checker> checker(noop_testing_callbacks{}, _Type::_Float32); + _Specs_checker> checker( + noop_testing_callbacks{}, _Basic_format_arg_type::_Float_type); (void) checker; return true; } From 9c6865de5afb3e56e4484d1009702e71487c0d36 Mon Sep 17 00:00:00 2001 From: Charlie Date: Wed, 17 Feb 2021 12:24:16 -0800 Subject: [PATCH 29/32] address code review comments --- stl/inc/format | 223 ++++++++++-------- .../test.cpp | 6 +- 2 files changed, 125 insertions(+), 104 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 41b34a57f0d..309bf64a38b 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -92,7 +92,7 @@ template concept _Parse_replacement_field_callbacks = requires(_Ty _At, const _CharT* _Begin, const _CharT* _End) { { _At._Parse_context }; { _At._On_text(_Begin, _End) } -> same_as; - { _At._On_replacement_field(size_t{}, _STD declval()) } -> same_as; + { _At._On_replacement_field(size_t{}, static_cast(nullptr)) } -> same_as; { _At._On_format_specs(size_t{}, _Begin, _End) } -> same_as; }; @@ -127,7 +127,7 @@ public: _Format_string.remove_prefix(_Diff); } - // While the standard presents an exposition only enum value for + // While the standard presents an exposition-only enum value for // the indexing mode (manual, automatic, or unknown) we use _Next_arg_id to indicate it. // _Next_arg_id > 0 means automatic // _Next_arg_id == 0 means unknown @@ -240,6 +240,43 @@ public: }; }; +template +auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { + switch (_Arg._Active_state) { + case _Basic_format_arg_type::_None: + return _Vis(_Arg._No_state); + case _Basic_format_arg_type::_Int_type: + return _Vis(_Arg._Int_state); + case _Basic_format_arg_type::_UInt_type: + return _Vis(_Arg._UInt_state); + case _Basic_format_arg_type::_Long_long_type: + return _Vis(_Arg._Long_long_state); + case _Basic_format_arg_type::_ULong_long_type: + return _Vis(_Arg._ULong_long_state); + case _Basic_format_arg_type::_Bool_type: + return _Vis(_Arg._Bool_state); + case _Basic_format_arg_type::_Char_type: + return _Vis(_Arg._Char_state); + case _Basic_format_arg_type::_Float_type: + return _Vis(_Arg._Float_state); + case _Basic_format_arg_type::_Double_type: + return _Vis(_Arg._Double_state); + case _Basic_format_arg_type::_Long_double_type: + return _Vis(_Arg._Long_double_state); + case _Basic_format_arg_type::_Pointer_type: + return _Vis(_Arg._Pointer_state); + case _Basic_format_arg_type::_CString_type: + return _Vis(_Arg._CString_state); + case _Basic_format_arg_type::_String_type: + return _Vis(_Arg._String_state); + case _Basic_format_arg_type::_Custom_type: + return _Vis(_Arg._Custom_state); + default: + _STL_VERIFY(false, "basic_format_arg is in impossible state"); + return _Vis(0); + } +} + // we need to implement this ourselves because from_chars does not work with wide characters template constexpr const _CharT* _Parse_nonnegative_integer(const _CharT* _Begin, const _CharT* _End, unsigned int& _Value) { @@ -301,7 +338,9 @@ constexpr const _CharT* _Parse_align(const _CharT* _Begin, const _CharT* _End, _ _STL_INTERNAL_CHECK(_Begin != _End && *_Begin != '}'); // align and fill auto _Parsed_align = _Align::_None; - auto _Align_pt = _Begin + 1; + + // TODO: should increment one code point + auto _Align_pt = _Begin + 1; if (_Align_pt == _End) { _Align_pt = _Begin; } @@ -535,56 +574,61 @@ template _HandlerT> constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _HandlerT&& _Handler) { auto _Begin = _Format_str.data(); auto _End = _Begin + _Format_str.size(); - struct _Writer { - constexpr void operator()(const _CharT* _Begin, const _CharT* _End) { - if (_Begin == _End) { - return; - } + while (_Begin != _End) { + const _CharT* _OpeningCurl = _Begin; + if (*_Begin != '{') { + // we didn't start at an opening curl, find the next one + _OpeningCurl = _STD find(_Begin + 1, _End, '{'); + _STL_INTERNAL_CHECK(_Begin != _OpeningCurl); for (;;) { - const _CharT* _Pt = _STD _Find_unchecked(_Begin, _End, '}'); - if (_Pt == _End) { - return _Handler._On_text(_Begin, _End); + const _CharT* _ClosingCurl = _STD _Find_unchecked(_Begin, _End, '}'); + + // In this case we didn't find either a closing curl or opening curl. + // Write the whole thing out. + if (_ClosingCurl == _OpeningCurl) { + return _Handler._On_text(_Begin, _OpeningCurl); } - ++_Pt; - if (_Pt == _End || *_Pt != '}') { + // We know _ClosingCurl isn't past the end because + // the above condition was not met. + ++_ClosingCurl; + if (_ClosingCurl == _OpeningCurl || *_ClosingCurl != '}') { throw format_error("Unmatched '}' in format string."); } - _Handler._On_text(_Begin, _Pt); - _Begin = _Pt + 1; + // We found two closing curls, so output online one of them + _Handler._On_text(_Begin, _ClosingCurl); + + // skip over the second closing curl + _Begin = _ClosingCurl + 1; } - } - _HandlerT& _Handler; - } _Writer_loop{_Handler}; - while (_Begin != _End) { - const _CharT* _Pt = _Begin; - if (*_Begin != '{') { - _Pt = _STD find(_Begin + 1, _End, '{'); - if (_Pt == _End) { - return _Writer_loop(_Begin, _End); + + // We are done, there were no replacement fields. + if (_OpeningCurl == _End) { + return; } } - _Writer_loop(_Begin, _Pt); - _Begin = _Parse_replacement_field(_Pt, _End, _Handler); + // Parse the replacement field starting at _OpeningCurl and ending sometime before _End. + _Begin = _Parse_replacement_field(_OpeningCurl, _End, _Handler); } } template struct _Basic_format_specs { - unsigned int _Width, _Precision; + unsigned int _Width; + unsigned int _Precision; char _Type; _Align _Alignment; _Sign _Sgn; bool _Alt; + // At most one codepoint (so one char32_t or four utf-8 char8_t). _CharT _Fill[4]; }; - // Model of _Parse_specs_callbacks that fills a _Basic_format_specs with the parsed data. template class _Specs_setter { public: - explicit constexpr _Specs_setter(_Basic_format_specs<_CharT> _Specs) : _Specs(_Specs) {} + explicit constexpr _Specs_setter(_Basic_format_specs<_CharT> _Specs_) : _Specs(_Specs_) {} constexpr void _On_align(_Align _Aln) { _Specs._Alignment = _Aln; @@ -592,9 +636,9 @@ public: constexpr void _On_fill(basic_string_view<_CharT> _Sv) { if (_Sv.size() > 4) { - throw format_error("Invalid fill."); + throw format_error("Invalid fill (too long)."); } - _STD fill(_Specs._Fill, _Specs._Fill + 4, '\0'); + _STD fill(_Specs._Fill, _Specs._Fill + 4, _CharT{}); _STD copy(_Sv.begin(), _Sv.end(), _Specs._Fill); } @@ -608,7 +652,7 @@ public: constexpr void _On_zero() { _Specs._Alignment = _Align::_None; - _Specs._Fill[0] = '0'; + _Specs._Fill[0] = _CharT{'0'}; } constexpr void _On_width(unsigned int _Width) { @@ -627,18 +671,6 @@ protected: _Basic_format_specs<_CharT> _Specs; }; -// Actually fetch the value of an argument associated with a dynamic -// width or precision specifier. This will be called with either -// _Width_checker or _Precision_checker as "_Handler". -template -constexpr unsigned int _Get_dynamic_specs(_FormatArg _Arg) { - unsigned long long _Val = visit_format_arg(_Handler(), _Arg); - if (_Val > (_STD numeric_limits::max)()) { - throw format_error("Number is too big."); - } - return static_cast(_Val); -} - template constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, size_t _Arg_id) { // note: while this is parameterized on the _Arg_id type in libfmt we don't @@ -652,7 +684,7 @@ constexpr basic_format_arg<_Context> _Get_arg(_Context _Ctx, size_t _Arg_id) { } // Checks that the type and value of an argument associated with a dynamic -// width specifier is valid. +// width specifier are valid. class _Width_checker { public: template @@ -664,13 +696,14 @@ public: } } return static_cast(_Value); + } else { + throw format_error("Width is not an integer."); } - throw format_error("Width is not an integer."); } }; // Checks that the type and value of an argument associated with a dynamic -// precision specifier is valid. +// precision specifier are valid. class _Precision_checker { public: template @@ -678,17 +711,18 @@ public: if constexpr (is_integral_v<_Ty>) { if constexpr (is_signed_v<_Ty>) { if (_Value < 0) { - throw format_error("Nagative precision."); + throw format_error("Negative precision."); } } return static_cast(_Value); + } else { + throw format_error("Precision is not an integer."); } - throw format_error("Precision is not an integer."); } }; // Parses standard format specs into a _Basic_format_specs using _Specs_setter, and, -// in addition handles dynamic width and precision. This is seperate from _Specs setter +// in addition handles dynamic width and precision. This is separate from _Specs setter // because it needs to know about the current basic_format_parse_context and basic_format_context // in order to fetch the width from the arguments. template @@ -696,8 +730,8 @@ class _Specs_handler : public _Specs_setter { public: using _CharT = typename _Context::char_type; - constexpr _Specs_handler(_Basic_format_specs<_CharT>& _Specs, _ParseContext& _Parse_ctx, _Context& _Ctx) - : _Specs_setter<_CharT>(_Specs), _Parse_ctx(_Parse_ctx), _Ctx(_Ctx) {} + constexpr _Specs_handler(_Basic_format_specs<_CharT>& _Specs_, _ParseContext& _Parse_ctx_, _Context& _Ctx_) + : _Specs_setter<_CharT>(_Specs_), _Parse_ctx(_Parse_ctx_), _Ctx(_Ctx_) {} template constexpr void _On_dynamic_width(_Id _Arg_id) { @@ -721,20 +755,34 @@ private: _Parse_ctx.check_arg_id(_Arg_id); return _STD _Get_arg(_Ctx, _Arg_id); } + + // Fetch the value of an argument associated with a dynamic + // width or precision specifier. This will be called with either + // _Width_checker or _Precision_checker as "_Handler". + template + static constexpr unsigned int _Get_dynamic_specs(_FormatArg _Arg) { + unsigned long long _Val = _STD visit_format_arg(_Handler(), _Arg); + if (_Val > (_STD numeric_limits::max)()) { + throw format_error("Number is too big."); + } + return static_cast(_Val); + } }; + class _Numeric_specs_checker { +private: _Basic_format_arg_type _Arg_type = _Basic_format_arg_type::_None; public: - constexpr _Numeric_specs_checker(_Basic_format_arg_type _Arg_type) : _Arg_type(_Arg_type) {} + constexpr explicit _Numeric_specs_checker(_Basic_format_arg_type _Arg_type_) : _Arg_type(_Arg_type_) {} - constexpr void _Require_numeric_argument() { + constexpr void _Require_numeric_argument() const { if (!_Is_arithmetic_fmt_type(_Arg_type)) { throw format_error("Format specifier requires numeric argument."); } } - constexpr void _Check_sign() { + constexpr void _Check_sign() const { _Require_numeric_argument(); if (_Is_integral_fmt_type(_Arg_type) && _Arg_type != _Basic_format_arg_type::_Int_type && _Arg_type != _Basic_format_arg_type::_Long_long_type @@ -743,7 +791,7 @@ public: } } - constexpr void _Check_precision() { + constexpr void _Check_precision() const { if (_Is_integral_fmt_type(_Arg_type) || _Arg_type == _Basic_format_arg_type::_Pointer_type) { throw format_error("Precision not allowed for this argument type."); } @@ -756,24 +804,31 @@ public: // handler method. template class _Specs_checker : public _Handler { +private: _Numeric_specs_checker _Numeric_checker; public: - constexpr _Specs_checker(const _Handler& _Handler_inst, _Basic_format_arg_type _Arg_type) - : _Handler(_Handler_inst), _Numeric_checker(_Arg_type) {} + constexpr explicit _Specs_checker(const _Handler& _Handler_inst, _Basic_format_arg_type _Arg_type_) + : _Handler(_Handler_inst), _Numeric_checker(_Arg_type_) {} // _On_align has no checking, since we don't implement numeric alignments. + constexpr void _On_sign(_Sign _Sgn) { _Numeric_checker._Check_sign(); _Handler::_On_sign(_Sgn); } constexpr void _On_hash() { + // Note that '#' is not valid for CharT or bool unless you + // pass a numeric presentation type, but we encounter '#' before + // the presentation type so we can not check that requirement here _Numeric_checker._Require_numeric_argument(); _Handler::_On_hash(); } constexpr void _On_zero() { + // Note 0 is again not valid for CharT or bool unless a numeric + // presentation type is uesd. _Numeric_checker._Require_numeric_argument(); _Handler::_On_zero(); } @@ -784,43 +839,6 @@ public: } }; -template -auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) { - switch (_Arg._Active_state) { - case _Basic_format_arg_type::_None: - return _Vis(_Arg._No_state); - case _Basic_format_arg_type::_Int_type: - return _Vis(_Arg._Int_state); - case _Basic_format_arg_type::_UInt_type: - return _Vis(_Arg._UInt_state); - case _Basic_format_arg_type::_Long_long_type: - return _Vis(_Arg._Long_long_state); - case _Basic_format_arg_type::_ULong_long_type: - return _Vis(_Arg._ULong_long_state); - case _Basic_format_arg_type::_Bool_type: - return _Vis(_Arg._Bool_state); - case _Basic_format_arg_type::_Char_type: - return _Vis(_Arg._Char_state); - case _Basic_format_arg_type::_Float_type: - return _Vis(_Arg._Float_state); - case _Basic_format_arg_type::_Double_type: - return _Vis(_Arg._Double_state); - case _Basic_format_arg_type::_Long_double_type: - return _Vis(_Arg._Long_double_state); - case _Basic_format_arg_type::_Pointer_type: - return _Vis(_Arg._Pointer_state); - case _Basic_format_arg_type::_CString_type: - return _Vis(_Arg._CString_state); - case _Basic_format_arg_type::_String_type: - return _Vis(_Arg._String_state); - case _Basic_format_arg_type::_Custom_type: - return _Vis(_Arg._Custom_state); - default: - _STL_VERIFY(false, "basic_format_arg is in impossible state"); - return _Vis(0); - } -} - template /* consteval */ constexpr auto _Get_format_arg_storage_type() noexcept { using _CharType = typename _Context::char_type; @@ -1068,8 +1086,8 @@ public: using formatter_type = formatter<_Ty, _CharT>; constexpr basic_format_context( - _Out _OutputIt, basic_format_args _Ctx_args, const locale& _Loc) - : _OutputIt(_OutputIt), _Args(_Ctx_args), _Loc(_Loc) {} + _Out _OutputIt_, basic_format_args _Ctx_args, const locale& _Loc_) + : _OutputIt(_OutputIt_), _Args(_Ctx_args), _Loc(_Loc_) {} basic_format_arg arg(size_t _Id) const { return _Args.get(_Id); @@ -1125,11 +1143,12 @@ private: _Context& _Ctx; public: - explicit constexpr _Custom_formatter_dispatcher(basic_format_parse_context<_Char_type>& _Parse_ctx, _Context& _Ctx) - : _Parse_ctx(_Parse_ctx), _Ctx(_Ctx) {} + explicit constexpr _Custom_formatter_dispatcher( + basic_format_parse_context<_Char_type>& _Parse_ctx_, _Context& _Ctx_) + : _Parse_ctx(_Parse_ctx_), _Ctx(_Ctx_) {} - void operator()(typename basic_format_arg<_Context>::handle h) const { - h.format(_Parse_ctx, _Ctx); + void operator()(typename basic_format_arg<_Context>::handle _Handle) const { + _Handle.format(_Parse_ctx, _Ctx); } template @@ -1174,7 +1193,7 @@ struct _Format_handler { void _On_text(const _CharT* _Begin, const _CharT* _End) { auto _Size = _End - _Begin; auto _Out = _Ctx.out(); - _STD copy_n(_Begin, _Size, _Out); + _Out = _STD copy_n(_Begin, _Size, _Out); _Ctx.advance_to(_Out); } diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index 7e28bdf95fb..3c0f0d9173d 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -3,12 +3,14 @@ #include #include +#include +#include #include #include // TODO: fill in tests -template std::back_insert_iterator std::vformat_to(std::back_insert_iterator, const locale&, - std::string_view, std::format_args_t, char>); +template std::back_insert_iterator std::vformat_to(std::back_insert_iterator, + const std::locale&, std::string_view, std::format_args_t, char>); int main() {} From d0e5304ceeaa56628b21df0824f4adf59d643b14 Mon Sep 17 00:00:00 2001 From: Charlie Date: Wed, 17 Feb 2021 12:28:25 -0800 Subject: [PATCH 30/32] alphabatize test.lst --- tests/std/test.lst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/test.lst b/tests/std/test.lst index 9b6d214e47e..d71e3493b8d 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -255,9 +255,9 @@ tests\P0616R0_using_move_in_numeric tests\P0631R8_numbers_math_constants tests\P0645R10_text_formatting_args tests\P0645R10_text_formatting_death +tests\P0645R10_text_formatting_formatting tests\P0645R10_text_formatting_parse_contexts tests\P0645R10_text_formatting_parsing -tests\P0645R10_text_formatting_formatting tests\P0660R10_jthread_and_cv_any tests\P0660R10_stop_token tests\P0660R10_stop_token_death From 3adac1c46bff84477ab31bad06fb83ce5edfe5a2 Mon Sep 17 00:00:00 2001 From: Charlie Date: Wed, 17 Feb 2021 12:35:11 -0800 Subject: [PATCH 31/32] static_cast(0) instead of unsigned{} --- stl/inc/format | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 309bf64a38b..0b5ab0f8b8f 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -71,10 +71,10 @@ template concept _Parse_spec_callbacks = requires(_Ty _At, basic_string_view<_CharT> _Sv, _Align _Aln, _Sign _Sgn) { { _At._On_align(_Aln) } -> same_as; { _At._On_fill(_Sv) } -> same_as; - { _At._On_width(unsigned{}) } -> same_as; + { _At._On_width(static_cast(0)) } -> same_as; { _At._On_dynamic_width(size_t{}) } -> same_as; { _At._On_dynamic_width(_Auto_id_tag{}) } -> same_as; - { _At._On_precision(unsigned{}) } -> same_as; + { _At._On_precision(static_cast(0)) } -> same_as; { _At._On_dynamic_precision(size_t{}) } -> same_as; { _At._On_dynamic_precision(_Auto_id_tag{}) } -> same_as; { _At._On_sign(_Sgn) } -> same_as; From 3715c9f77e45e44b0c47e73d9346000b73c0162a Mon Sep 17 00:00:00 2001 From: Charlie Date: Thu, 18 Feb 2021 12:19:07 -0800 Subject: [PATCH 32/32] remove dead _STL_INTERNAL_CHECK and find in the correct range --- stl/inc/format | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 0b5ab0f8b8f..65b9145847a 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -123,6 +123,7 @@ public: } constexpr void advance_to(const const_iterator _It) { _Adl_verify_range(_It, _Format_string.end()); + // _It must be after _Format_string.begin(). const auto _Diff = static_cast(_It._Unwrapped() - _Format_string._Unchecked_begin()); _Format_string.remove_prefix(_Diff); } @@ -579,9 +580,8 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand if (*_Begin != '{') { // we didn't start at an opening curl, find the next one _OpeningCurl = _STD find(_Begin + 1, _End, '{'); - _STL_INTERNAL_CHECK(_Begin != _OpeningCurl); for (;;) { - const _CharT* _ClosingCurl = _STD _Find_unchecked(_Begin, _End, '}'); + const _CharT* _ClosingCurl = _Find_unchecked(_Begin, _OpeningCurl, '}'); // In this case we didn't find either a closing curl or opening curl. // Write the whole thing out. @@ -594,7 +594,7 @@ constexpr void _Parse_format_string(basic_string_view<_CharT> _Format_str, _Hand if (_ClosingCurl == _OpeningCurl || *_ClosingCurl != '}') { throw format_error("Unmatched '}' in format string."); } - // We found two closing curls, so output online one of them + // We found two closing curls, so output only one of them _Handler._On_text(_Begin, _ClosingCurl); // skip over the second closing curl @@ -762,7 +762,7 @@ private: template static constexpr unsigned int _Get_dynamic_specs(_FormatArg _Arg) { unsigned long long _Val = _STD visit_format_arg(_Handler(), _Arg); - if (_Val > (_STD numeric_limits::max)()) { + if (_Val > (numeric_limits::max)()) { throw format_error("Number is too big."); } return static_cast(_Val);