From 5191e21a950565fe24cff214cc3eff5260eda14b Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 20:27:50 +0100 Subject: [PATCH 01/36] Noralize next_arg_id --- stl/inc/format | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 16466b6980d..86d734bc142 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -343,13 +343,12 @@ public: // _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 (_Next_arg_id == -1) { + throw format_error("Can not switch from manual to automatic indexing"); } - throw format_error("Can not switch from manual to automatic indexing"); + return _Next_arg_id++; } - constexpr void check_arg_id(size_t _Id) { - (void) _Id; + constexpr void check_arg_id(size_t) { if (_Next_arg_id > 0) { throw format_error("Can not switch from automatic to manual indexing"); } From 6a4ff26e7b21e9fb105c073e24f64b3b384474b1 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 20:35:37 +0100 Subject: [PATCH 02/36] Add precondition check to basic_format_parse_context::advance_to and debug optimization --- stl/inc/format | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 86d734bc142..fbb34356ed5 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -333,8 +333,9 @@ public: _NODISCARD constexpr const_iterator end() const noexcept { return _Format_string.end(); } - constexpr void advance_to(const_iterator _It) { - _Format_string.remove_prefix(_It - begin()); + constexpr void advance_to(const const_iterator _It) { + _Adl_verify_range(_It, _Format_string.end()); + _Format_string.remove_prefix(_It - _Format_string.begin()); } // While the standard presents an exposition only enum value for From be1afc032633a78911e9d93b2ad4b2fc461d041f Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 20:41:31 +0100 Subject: [PATCH 03/36] ensure basic_format_parse_context::check_arg_id is not a constant expression --- stl/inc/format | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index fbb34356ed5..be583cac137 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -349,7 +349,15 @@ public: } return _Next_arg_id++; } - constexpr void check_arg_id(size_t) { + constexpr void check_arg_id(const size_t _Id) { + if (_STD is_constant_evaluated()) { + if (_Id >= _Num_args) { + _TRY_BEGIN + throw 42; + _CATCH(42) + _CATCH_END + } + } if (_Next_arg_id > 0) { throw format_error("Can not switch from automatic to manual indexing"); } From ae417d7f2687a697f0f14c8e83beba1a69875efc Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 21:51:08 +0100 Subject: [PATCH 04/36] Add tests for basic_format_parse_context --- stl/inc/format | 9 +- tests/std/test.lst | 1 + .../P0645R10_text_formatting_death/env.lst | 4 + .../P0645R10_text_formatting_death/test.cpp | 30 ++++++ .../test.cpp | 91 ++++++++++++++++++- 5 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 tests/std/tests/P0645R10_text_formatting_death/env.lst create mode 100644 tests/std/tests/P0645R10_text_formatting_death/test.cpp diff --git a/stl/inc/format b/stl/inc/format index be583cac137..f1faadc5e64 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -314,6 +314,8 @@ public: using iterator = const_iterator; private: + using _Size_type = typename basic_string_view<_CharT>::size_type; + 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 @@ -335,7 +337,8 @@ public: } constexpr void advance_to(const const_iterator _It) { _Adl_verify_range(_It, _Format_string.end()); - _Format_string.remove_prefix(_It - _Format_string.begin()); + const auto _Diff = static_cast<_Size_type>(_It - _Format_string.begin()); + _Format_string.remove_prefix(_Diff); } // While the standard presents an exposition only enum value for @@ -347,14 +350,14 @@ public: if (_Next_arg_id == -1) { throw format_error("Can not switch from manual to automatic indexing"); } - return _Next_arg_id++; + return static_cast(_Next_arg_id++); } constexpr void check_arg_id(const size_t _Id) { if (_STD is_constant_evaluated()) { if (_Id >= _Num_args) { _TRY_BEGIN throw 42; - _CATCH(42) + _CATCH_ALL _CATCH_END } } diff --git a/tests/std/test.lst b/tests/std/test.lst index de1a48a5e19..7d5f3a8123d 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -243,6 +243,7 @@ tests\P0595R2_is_constant_evaluated tests\P0607R0_inline_variables tests\P0616R0_using_move_in_numeric tests\P0631R8_numbers_math_constants +tests\P0645R10_text_formatting_death tests\P0645R10_text_formatting_parse_contexts tests\P0645R10_text_formatting_parsing tests\P0660R10_jthread_and_cv_any diff --git a/tests/std/tests/P0645R10_text_formatting_death/env.lst b/tests/std/tests/P0645R10_text_formatting_death/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_death/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_death/test.cpp b/tests/std/tests/P0645R10_text_formatting_death/test.cpp new file mode 100644 index 00000000000..86e74da6037 --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_death/test.cpp @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#define _CONTAINER_DEBUG_LEVEL 1 + +#include +#include +#include + +#include +using namespace std; + +void test_case_advance_no_range() { + const auto format_string = "First {} and second {} and third {}"sv; + const auto other_format_string = "something"sv; + basic_format_parse_context context{format_string}; + context.advance_to(other_format_string.begin()); +} + +int main(int argc, char* argv[]) { + std_testing::death_test_executive exec; + +#if _ITERATOR_DEBUG_LEVEL != 0 + exec.add_death_tests({ + test_case_advance_no_range, + }); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + + return exec.run(argc, argv); +} diff --git a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp index 7a3f5c0dad2..6d49094c194 100644 --- a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp @@ -2,11 +2,98 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include +#include #include #include -// TODO: fill in tests +using namespace std; +using namespace literals; + +template +constexpr auto get_input() { + if constexpr (same_as) { + return "First {} and second {} and third {}"sv; + } else { + return L"First {} and second {} and third {}"sv; + } +} + +template +bool ensure_not_constant_expression() { + basic_format_parse_context context{get_input()}; + context.check_arg_id(1); + if (is_constant_evaluated()) { + return false; + } else { + return true; + } +} + +const bool check_arg_id_not_constexpr_char = ensure_not_constant_expression(); +const bool check_arg_id_not_constexpr_wchar = ensure_not_constant_expression(); + +template +constexpr bool test_basic_format_parse_context() { + static_assert(!is_copy_constructible_v>); + static_assert(!is_copy_assignable_v>); + + const auto format_string = get_input(); + { // iterator interface + basic_format_parse_context context{format_string}; + const same_as::const_iterator> auto b = context.begin(); + assert(b == format_string.begin()); + static_assert(noexcept(context.begin())); + + const same_as::const_iterator> auto e = context.end(); + assert(e == format_string.end()); + static_assert(noexcept(context.end())); + + const auto new_position = b + 5; + context.advance_to(new_position); + assert(to_address(context.begin()) == to_address(new_position)); + assert(to_address(context.end()) == to_address(e)); + } + + { // arg_id + basic_format_parse_context context{format_string}; + const same_as auto first_arg_id = context.next_arg_id(); + assert(first_arg_id == 0); + + const same_as auto second_arg_id = context.next_arg_id(); + assert(second_arg_id == 1); + + if (!is_constant_evaluated()) { + try { + context.check_arg_id(0); + } catch (format_error e) { + assert(e.what() == "Can not switch from automatic to manual indexing"sv); + } + } + } + + { // check_arg_id + basic_format_parse_context context{format_string, 3}; + context.check_arg_id(1); + context.check_arg_id(1); + + if (!is_constant_evaluated()) { + try { + context.next_arg_id(); + } catch (format_error e) { + assert(e.what() == "Can not switch from manual to automatic indexing"sv); + } + } + } + + return true; +} int main() { - return 0; + test_basic_format_parse_context(); + test_basic_format_parse_context(); + static_assert(test_basic_format_parse_context()); + static_assert(test_basic_format_parse_context()); + + assert(check_arg_id_not_constexpr_char); + assert(check_arg_id_not_constexpr_wchar); } From cbea0a50bdf1c15ffcd67d9e82f64cedac897289 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 22:09:02 +0100 Subject: [PATCH 05/36] Fix constant expression check --- .../test.cpp | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp index 6d49094c194..1a8b59c9fa9 100644 --- a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp @@ -19,18 +19,24 @@ constexpr auto get_input() { } template -bool ensure_not_constant_expression() { - basic_format_parse_context context{get_input()}; - context.check_arg_id(1); - if (is_constant_evaluated()) { - return false; +constexpr bool ensure_is_constant_expression(const bool should_be_constant_expression) { + basic_format_parse_context context{get_input(), 1}; + if (should_be_constant_expression) { + context.check_arg_id(0); } else { + context.check_arg_id(1); + } + if (is_constant_evaluated()) { return true; + } else { + return false; } } -const bool check_arg_id_not_constexpr_char = ensure_not_constant_expression(); -const bool check_arg_id_not_constexpr_wchar = ensure_not_constant_expression(); +const bool check_arg_id_is_constexpr_char = ensure_is_constant_expression(true); +const bool check_arg_id_not_constexpr_char = not ensure_is_constant_expression(false); +const bool check_arg_id_is_constexpr_wchar = ensure_is_constant_expression(true); +const bool check_arg_id_not_constexpr_wchar = not ensure_is_constant_expression(false); template constexpr bool test_basic_format_parse_context() { @@ -94,6 +100,8 @@ int main() { static_assert(test_basic_format_parse_context()); static_assert(test_basic_format_parse_context()); + assert(check_arg_id_is_constexpr_char); assert(check_arg_id_not_constexpr_char); + assert(check_arg_id_is_constexpr_wchar); assert(check_arg_id_not_constexpr_wchar); } From ba63570faa80b5b09675baba1b8da1fffe88b0fc Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 22:31:29 +0100 Subject: [PATCH 06/36] Not not --- .../tests/P0645R10_text_formatting_parse_contexts/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp index 1a8b59c9fa9..08c0e777a63 100644 --- a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp @@ -34,9 +34,9 @@ constexpr bool ensure_is_constant_expression(const bool should_be_constant_expre } const bool check_arg_id_is_constexpr_char = ensure_is_constant_expression(true); -const bool check_arg_id_not_constexpr_char = not ensure_is_constant_expression(false); +const bool check_arg_id_not_constexpr_char = !ensure_is_constant_expression(false); const bool check_arg_id_is_constexpr_wchar = ensure_is_constant_expression(true); -const bool check_arg_id_not_constexpr_wchar = not ensure_is_constant_expression(false); +const bool check_arg_id_not_constexpr_wchar = !ensure_is_constant_expression(false); template constexpr bool test_basic_format_parse_context() { From 816fe40eab67d5556995c093f3e950cd121c1b7c Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 29 Jan 2021 22:32:58 +0100 Subject: [PATCH 07/36] windsdk is bad --- tests/std/tests/P0645R10_text_formatting_death/env.lst | 2 +- tests/std/tests/P0645R10_text_formatting_parse_contexts/env.lst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_death/env.lst b/tests/std/tests/P0645R10_text_formatting_death/env.lst index f3ccc8613c6..22f1f0230a4 100644 --- a/tests/std/tests/P0645R10_text_formatting_death/env.lst +++ b/tests/std/tests/P0645R10_text_formatting_death/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\concepts_matrix.lst +RUNALL_INCLUDE ..\strict_winsdk_concepts_matrix.lst diff --git a/tests/std/tests/P0645R10_text_formatting_parse_contexts/env.lst b/tests/std/tests/P0645R10_text_formatting_parse_contexts/env.lst index f3ccc8613c6..22f1f0230a4 100644 --- a/tests/std/tests/P0645R10_text_formatting_parse_contexts/env.lst +++ b/tests/std/tests/P0645R10_text_formatting_parse_contexts/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\concepts_matrix.lst +RUNALL_INCLUDE ..\strict_winsdk_concepts_matrix.lst From 5485658cfe41d1e7d55463f5cb011e52574c4c7f Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 30 Jan 2021 12:45:12 +0100 Subject: [PATCH 08/36] Address review comments --- stl/inc/format | 29 ++++++++++--------- .../test.cpp | 20 +++++++------ 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index f1faadc5e64..1c89620f3ad 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -305,7 +305,6 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* template struct formatter; -// TODO: test coverage template class basic_format_parse_context { public: @@ -313,17 +312,6 @@ public: using const_iterator = typename basic_string_view<_CharT>::const_iterator; using iterator = const_iterator; -private: - using _Size_type = typename basic_string_view<_CharT>::size_type; - - 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; @@ -337,7 +325,7 @@ public: } constexpr void advance_to(const const_iterator _It) { _Adl_verify_range(_It, _Format_string.end()); - const auto _Diff = static_cast<_Size_type>(_It - _Format_string.begin()); + const auto _Diff = static_cast(_It._Unwrapped() - _Format_string._Unchecked_begin()); _Format_string.remove_prefix(_Diff); } @@ -350,22 +338,37 @@ public: if (_Next_arg_id == -1) { 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) { +#ifdef __clang__ // TRANSITION clang-12 _TRY_BEGIN throw 42; _CATCH_ALL _CATCH_END +#else // ^^^ workaround ^^^ / vvv no workaround vvv + [[maybe_unused]] const auto _Reinterpret_to_make_not_constexpr = reinterpret_cast(&_Num_args); +#endif // !__clang__ } } + 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. Below is a more detailed explanation + // of how this works. + ptrdiff_t _Next_arg_id = 0; }; // TODO: test coverage diff --git a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp index 08c0e777a63..606f08b51f4 100644 --- a/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_parse_contexts/test.cpp @@ -3,11 +3,13 @@ #include #include +#include #include +#include #include +#include using namespace std; -using namespace literals; template constexpr auto get_input() { @@ -26,11 +28,8 @@ constexpr bool ensure_is_constant_expression(const bool should_be_constant_expre } else { context.check_arg_id(1); } - if (is_constant_evaluated()) { - return true; - } else { - return false; - } + + return is_constant_evaluated(); } const bool check_arg_id_is_constexpr_char = ensure_is_constant_expression(true); @@ -54,7 +53,7 @@ constexpr bool test_basic_format_parse_context() { assert(e == format_string.end()); static_assert(noexcept(context.end())); - const auto new_position = b + 5; + const auto new_position = format_string.begin() + 5; context.advance_to(new_position); assert(to_address(context.begin()) == to_address(new_position)); assert(to_address(context.end()) == to_address(e)); @@ -71,7 +70,7 @@ constexpr bool test_basic_format_parse_context() { if (!is_constant_evaluated()) { try { context.check_arg_id(0); - } catch (format_error e) { + } catch (const format_error& e) { assert(e.what() == "Can not switch from automatic to manual indexing"sv); } } @@ -79,13 +78,16 @@ constexpr bool test_basic_format_parse_context() { { // check_arg_id basic_format_parse_context context{format_string, 3}; + context.check_arg_id(0); context.check_arg_id(1); + context.check_arg_id(2); // intentional duplicates to check whether this is ok to call multiple times context.check_arg_id(1); + context.check_arg_id(0); if (!is_constant_evaluated()) { try { context.next_arg_id(); - } catch (format_error e) { + } catch (const format_error& e) { assert(e.what() == "Can not switch from manual to automatic indexing"sv); } } From e938cfaad905e2a244f958b228ab33cc50b7f3c6 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 31 Jan 2021 12:06:00 +0100 Subject: [PATCH 09/36] Use a non constexpr function call with a better name Co-authored-by: S. B. Tam --- stl/inc/format | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 1c89620f3ad..22b8daefc60 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -305,6 +305,8 @@ constexpr const _CharT* _Parse_format_specs(const _CharT* _Begin, const _CharT* template struct formatter; +inline void _You_see_this_error_because_arg_id_is_out_of_range() noexcept {} + template class basic_format_parse_context { public: @@ -345,14 +347,7 @@ public: constexpr void check_arg_id(const size_t _Id) { if (_STD is_constant_evaluated()) { if (_Id >= _Num_args) { -#ifdef __clang__ // TRANSITION clang-12 - _TRY_BEGIN - throw 42; - _CATCH_ALL - _CATCH_END -#else // ^^^ workaround ^^^ / vvv no workaround vvv - [[maybe_unused]] const auto _Reinterpret_to_make_not_constexpr = reinterpret_cast(&_Num_args); -#endif // !__clang__ + _You_see_this_error_because_arg_id_is_out_of_range(); } } From 31b15992612802cf7eb081e8489e54960cb6abcf Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Tue, 2 Feb 2021 14:16:02 +0100 Subject: [PATCH 10/36] Create a packed _Format_arg_store --- stl/inc/format | 379 +++++++++++++++++- tests/std/test.lst | 1 + .../P0645R10_text_formatting_args/env.lst | 4 + .../P0645R10_text_formatting_args/test.cpp | 226 +++++++++++ 4 files changed, 588 insertions(+), 22 deletions(-) create mode 100644 tests/std/tests/P0645R10_text_formatting_args/env.lst create mode 100644 tests/std/tests/P0645R10_text_formatting_args/test.cpp diff --git a/stl/inc/format b/stl/inc/format index 22b8daefc60..b1268d49693 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -366,58 +366,393 @@ private: ptrdiff_t _Next_arg_id = 0; }; -// TODO: test coverage +enum class _Basic_format_arg_type : uint8_t { + _None, + _Int_type, + _UInt_type, + _Long_type, + _ULong_type, + //_Int128_type, + //_UInt128_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: - class handle; - -private: - using _Char_type = typename _Context::char_type; + using _CharType = typename _Context::char_type; - using _Format_arg_value = variant, handle>; + template + friend auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)); - _Format_arg_value _Value; - -public: class handle { private: const void* _Ptr; - void (*_Format)(basic_format_parse_context<_Char_type>& _Parse_ctx, _Context _Format_ctx, const void*); + void (*_Format)(basic_format_parse_context<_CharType>& _Parse_ctx, _Context _Format_ctx, const void*); friend basic_format_arg; public: - void format(basic_format_parse_context<_Char_type>& _Parse_ctx, _Context& _Format_ctx) { + 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> _Fn; + _Parse_ctx.advance_to(_Fn.parse(_Parse_ctx)); + _Format_ctx.advance_to(_Fn.format(*static_cast(_Ptr), _Format_ctx)); + }){}; + + void format(basic_format_parse_context<_CharType>& _Parse_ctx, _Context& _Format_ctx) { _Format(_Parse_ctx, _Format_ctx, _Ptr); } }; - basic_format_arg() noexcept = default; + 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 _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_type), _Long_state(_Val) {} + explicit basic_format_arg(const unsigned long long _Val) noexcept + : _Active_state(_Basic_format_arg_type::_ULong_type), _ULong_state(_Val) {} + // explicit basic_format_arg(const __int128 _Val) noexcept + // : _Active_state(_Basic_format_arg_type::_Int128_type), _Int128_state(_Val) {} + // explicit basic_format_arg(const __uint128 _Val) noexcept + // : _Active_state(_Basic_format_arg_type::_UInt128_type), _UInt128_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), _LDouble_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 !_STD holds_alternative(_Value); + return _Active_state != _Basic_format_arg_type::_None; } + +private: + _Basic_format_arg_type _Active_state = _Basic_format_arg_type::_None; + union { + monostate _No_state = monostate{}; + int _Int_state; + unsigned _UInt_state; + long long _Long_state; + unsigned long long _ULong_state; + // __int128 _Int128_state; + // __uint128 _UInt128_state; + bool _Bool_state; + _CharType _Char_state; + float _Float_state; + double _Double_state; + long double _LDouble_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) -> decltype(_Vis(0)) { + 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_type: + return _Vis(_Arg._Long_state); + case _Basic_format_arg_type::_ULong_type: + return _Vis(_Arg._ULong_state); + // case _Basic_format_arg_type::_Int128_type: + // return _Vis(_Arg._Int128_state); + // case _Basic_format_arg_type::_UInt128_type: + // return _Vis(_Arg._UInt128_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._LDouble_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_ASSERT(false, "Invalid basic_format_arg type"); + return _Vis(0); + } +} + +template +static constexpr auto _Get_format_arg_storage_type() noexcept { + using _CharType = typename _Context::char_type; + if constexpr (is_same_v<_Ty, monostate>) { + return monostate{}; + } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { + return int{}; + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned)) { + return unsigned{}; + } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { + return static_cast(42); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { + return static_cast(42); + //} else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(__int128)) { + // return __int128{42}; + //} else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(__unint128)) { + // return __uint128{42}; + } else if constexpr (is_same_v<_Ty, bool>) { + return true; + } else if constexpr (is_same_v<_Ty, _CharType>) { + return _CharType{}; + } else if constexpr (is_same_v<_Ty, float>) { + return float{}; + } else if constexpr (is_same_v<_Ty, double>) { + return double{}; + } else if constexpr (is_same_v<_Ty, long double>) { + return static_cast(42); + } else if constexpr (is_same_v<_Ty, const void*>) { + return static_cast(nullptr); + } else if constexpr (is_same_v<_Ty, const _CharType*>) { + return static_cast(nullptr); + } else if constexpr (is_same_v<_Ty, basic_string_view<_CharType>>) { + return basic_string_view<_CharType>{}; + } else { + return basic_format_arg<_Context>::handle(); + } +} + +template +static constexpr size_t _Get_format_arg_storage_size() noexcept { + return sizeof(_Get_format_arg_storage_type<_Context, _Ty>()); +} + +struct _Format_arg_store_packed_index { + // TRANSITION, Should be templated on number of arguments for even less storage + using _Index_type = size_t; + + constexpr _Format_arg_store_packed_index() = default; + constexpr _Format_arg_store_packed_index(const size_t _Index) + : _Index(static_cast<_Index_type>(_Index)), _Type(_Basic_format_arg_type::_None) {} + + _Index_type _Index : (sizeof(_Index_type) - 4); + _Basic_format_arg_type _Type : 4; }; -// TODO: test coverage template class _Format_arg_store { - static constexpr size_t _Num_args = sizeof...(_Args); - // TODO: no arg packing yet +private: + template + friend class basic_format_args; + + static constexpr size_t _Num_args = sizeof...(_Args); + static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args>() + ... + 0); + + using _CharType = typename _Context::char_type; + using _Index_type = _Format_arg_store_packed_index; + + template + void _Store_impl(const size_t _Arg_index, _Ty _Val) noexcept { + const auto _Store_index = _Index_array[_Arg_index]._Index; + const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>(); + + std::memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); + _Index_array[_Arg_index]._Type = _Arg_type; + if (_Arg_index < _Num_args) { + _Index_array[_Arg_index + 1] = _Format_arg_store_packed_index{_Store_index + _Length}; + } + } + + byte _Storage[(_STD max)(_Storage_length, 1ull)]; + _Index_type _Index_array[(_STD max)(_Num_args, 1ull)]; + +public: + // See [format.arg]/5 + template + void _Store(const size_t _Arg_index, const _Ty& _Val) noexcept { + if constexpr (_Same_impl<_Ty, bool>) { + _Store_impl(_Arg_index, _Val); + } else if constexpr (_Same_impl<_Ty, _CharType>) { + _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, _Val); + } else if constexpr (_Same_impl<_Ty, char> && _Same_impl<_CharType, wchar_t>) { + _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, static_cast(_Val)); + } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { + _Store_impl(_Arg_index, static_cast(_Val)); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned)) { + _Store_impl(_Arg_index, static_cast(_Val)); + } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { + _Store_impl(_Arg_index, static_cast(_Val)); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { + _Store_impl( + _Arg_index, static_cast(_Val)); + } else { + _Store_impl::handle, _Basic_format_arg_type::_Custom_type>( + _Arg_index, basic_format_arg<_Context>::handle(_Val)); + } + } + + void _Store(const size_t _Arg_index, float _Val) noexcept { + _Store_impl(_Arg_index, _Val); + } + + void _Store(const size_t _Arg_index, double _Val) noexcept { + _Store_impl(_Arg_index, _Val); + } - using _Value_type = basic_format_arg<_Context>; + void _Store(const size_t _Arg_index, long double _Val) noexcept { + _Store_impl(_Arg_index, _Val); + } + + void _Store(const size_t _Arg_index, const _CharType* _Val) noexcept { + _Store_impl(_Arg_index, _Val); + } + + template + void _Store(const size_t _Arg_index, basic_string_view<_CharType, _Traits> _Val) noexcept { + _Store_impl, _Basic_format_arg_type::_String_type>( + _Arg_index, basic_string_view<_CharType>{_Val}); + } + + template + void _Store(const size_t _Arg_index, basic_string<_CharType, _Traits, _Alloc> _Val) noexcept { + _Store_impl, _Basic_format_arg_type::_String_type>( + _Arg_index, _Basic_format_arg_type::_String_type, basic_string_view<_CharType>{_Val.data(), _Val.size()}); + } + + void _Store(const size_t _Arg_index, nullptr_t) noexcept { + _Store_impl(_Arg_index, static_cast(nullptr)); + } + + // clang-format off + template + requires is_void_v<_Ty> + void _Store(const size_t _Arg_index, const _Ty* p) noexcept { + // clang-format on + _Store_impl(_Arg_index, static_cast(p)); + } }; -// TODO: test coverage +// TRANSITION, Contraints missing, default _Context missing +template +_Format_arg_store<_Context> make_format_args() { + return _Format_arg_store<_Context>{}; +} + +template +_Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { + _Format_arg_store<_Context, _Args...> _Arg_store{}; + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + return _Arg_store; +} + +// TRANSITION, Contraints missing, WContext missing +template +_Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { + _Format_arg_store<_Context, _Args...> _Arg_store{}; + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + return _Arg_store; +} + template class basic_format_args { +private: + using _CharType = typename _Context::char_type; + + template + _NODISCARD static constexpr auto _Get_value_from_memory(const byte* _Storage) noexcept { + byte _Temp[sizeof(_Ty)]; + _Copy_unchecked(_Storage, _Storage + sizeof(_Ty), _Temp); + return _Bit_cast<_Ty>(_Temp); + } + public: basic_format_args() noexcept; template - basic_format_args(const _Format_arg_store<_Context, _Args...>& store) noexcept; + basic_format_args(const _Format_arg_store<_Context, _Args...>& store) noexcept + : _Num_args(sizeof...(_Args)), _Storage(store._Storage), _Index_array(store._Index_array) {} + + basic_format_arg<_Context> get(const size_t _Index) const noexcept { + if (_Index >= _Num_args) { + return basic_format_arg<_Context>{}; + } - basic_format_arg<_Context> get(size_t _Index) const noexcept; + const byte* _Arg_storage = _Storage + _Index_array[_Index]._Index; + switch (_Index_array[_Index]._Type) { + case _Basic_format_arg_type::_None: + return basic_format_arg<_Context>{}; + case _Basic_format_arg_type::_Int_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_UInt_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_Long_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_ULong_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + // case _Basic_format_arg_type::_Int128_type: + // return basic_format_arg<_Context>{_Get_value_from_memory<__int128>(_Arg_storage)}; + // case _Basic_format_arg_type::_UInt128_type: + // return basic_format_arg<_Context>{_Get_value_from_memory<__uint128>(_Arg_storage)}; + case _Basic_format_arg_type::_Bool_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_Char_type: + return basic_format_arg<_Context>{_Get_value_from_memory<_CharType>(_Arg_storage)}; + case _Basic_format_arg_type::_Float_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_Double_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_Long_double_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_Pointer_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_CString_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_String_type: + return basic_format_arg<_Context>{_Get_value_from_memory>(_Arg_storage)}; + case _Basic_format_arg_type::_Custom_type: + return basic_format_arg<_Context>{ + _Get_value_from_memory::handle>(_Arg_storage)}; + default: + _STL_ASSERT(false, "Invalid basic_format_arg type"); + return basic_format_arg<_Context>{}; + } + } + +private: + size_t _Num_args = 0; + const byte* _Storage = nullptr; + const _Format_arg_store_packed_index* _Index_array = nullptr; }; // TODO: test coverage diff --git a/tests/std/test.lst b/tests/std/test.lst index 7d5f3a8123d..571ae196a8a 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -243,6 +243,7 @@ tests\P0595R2_is_constant_evaluated tests\P0607R0_inline_variables 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_parse_contexts tests\P0645R10_text_formatting_parsing diff --git a/tests/std/tests/P0645R10_text_formatting_args/env.lst b/tests/std/tests/P0645R10_text_formatting_args/env.lst new file mode 100644 index 00000000000..22f1f0230a4 --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_args/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\strict_winsdk_concepts_matrix.lst diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp new file mode 100644 index 00000000000..42895a64d6d --- /dev/null +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -0,0 +1,226 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; + +#pragma warning(push) +#pragma warning(disable : 4582) // 'uninit_vector::storage': constructor is not implicitly called +#pragma warning(disable : 4583) // 'uninit_vector::storage': destructor is not implicitly called + +template +constexpr auto get_input_literal() { + if constexpr (same_as) { + return "meow"; + } else { + return L"meow"; + } +} + +template +constexpr auto get_input_sv() { + if constexpr (same_as) { + return "meow"sv; + } else { + return L"meow"sv; + } +} + +class context { +public: + using char_type = char; +}; + +class wcontext { +public: + using char_type = wchar_t; +}; + +enum class Arg_type : uint8_t { + none, + int_type, + unsigned_type, + long_type, + unsigned_long_type, + bool_type, + char_type, + float_type, + double_type, + long_double_type, + pointer_type, + string_literal_type, + string_type, + handle_type, +}; + +template +auto visitor = [](auto&& arg) { + using T = decay_t; + using char_type = typename Context::char_type; + if constexpr (is_same_v) { + return Arg_type::none; + } else if constexpr (is_same_v) { + return Arg_type::int_type; + } else if constexpr (is_same_v) { + return Arg_type::unsigned_type; + } else if constexpr (is_same_v) { + return Arg_type::long_type; + } else if constexpr (is_same_v) { + return Arg_type::unsigned_long_type; + } else if constexpr (is_same_v) { + return Arg_type::char_type; + } else if constexpr (is_same_v) { + return Arg_type::float_type; + } else if constexpr (is_same_v) { + return Arg_type::double_type; + } else if constexpr (is_same_v) { + return Arg_type::long_double_type; + } else if constexpr (is_same_v) { + return Arg_type::pointer_type; + } else if constexpr (is_same_v) { + return Arg_type::string_literal_type; + } else if constexpr (is_same_v>) { + return Arg_type::string_type; + } else { + return Arg_type::handle_type; + } +}; + +template +void test_basic_format_arg() { + using char_type = typename Context::char_type; + + { // construction + basic_format_arg default_constructed; + assert(!default_constructed); + + basic_format_arg from_int{5}; + assert(from_int); + + basic_format_arg from_unsigned{5u}; + assert(from_unsigned); + + basic_format_arg from_long_long{5ll}; + assert(from_long_long); + + basic_format_arg from_unsigned_long_long{5ull}; + assert(from_unsigned_long_long); + + basic_format_arg from_float{5.0f}; + assert(from_float); + + basic_format_arg from_double{5.0}; + assert(from_double); + + basic_format_arg from_long_double{static_cast(5.0)}; + assert(from_long_double); + + basic_format_arg from_pointer{static_cast(nullptr)}; + assert(from_pointer); + + basic_format_arg from_literal{get_input_literal()}; + assert(from_literal); + + basic_format_arg from_string_view{get_input_sv()}; + assert(from_string_view); + + // TRANSITION, implement handle + // basic_format_arg from_handle{}; + // assert(from_handle); + } +} +template +void test_empty_format_arg_int() { + const auto store = make_format_args(); + const basic_format_args args{store}; + const same_as> auto first_arg = args.get(0); + assert(!first_arg); +} + +template +void test_single_format_arg(Type value) { + const auto store = make_format_args(value); + const basic_format_args args{store}; + const same_as> auto first_arg = args.get(0); + assert(first_arg); + assert(visit_format_arg(visitor, first_arg) == Result); + const same_as> auto other_arg = args.get(1); + assert(!other_arg); +} + +template +void test_format_arg_store() { + using char_type = typename Context::char_type; + + test_empty_format_arg_int(); + + if constexpr (is_same_v) { + test_single_format_arg(42); + test_single_format_arg(42); + } else { + test_single_format_arg(42); + test_single_format_arg(42); + } + test_single_format_arg(42); + test_single_format_arg(42); +#ifdef __cpp_char8_t + test_single_format_arg(42); +#endif // __cpp_char8_t + test_single_format_arg(42); + test_single_format_arg(42); + + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + + test_single_format_arg(42.f); + test_single_format_arg(42.); + test_single_format_arg(42.); + + test_single_format_arg(nullptr); + + test_single_format_arg(get_input_literal()); + test_single_format_arg, Arg_type::string_type>(get_input_sv()); +} + +int main() { + test_basic_format_arg(); + test_basic_format_arg(); + test_format_arg_store(); + test_format_arg_store(); +} +#pragma warning(pop) From 94f71091135f71dc43309b37ebc93395ccec236f Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 14:41:42 +0100 Subject: [PATCH 11/36] We want bits not bytes --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index b1268d49693..fabcda6ec24 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -565,7 +565,7 @@ struct _Format_arg_store_packed_index { constexpr _Format_arg_store_packed_index(const size_t _Index) : _Index(static_cast<_Index_type>(_Index)), _Type(_Basic_format_arg_type::_None) {} - _Index_type _Index : (sizeof(_Index_type) - 4); + _Index_type _Index : (sizeof(_Index_type) * 8 - 4); _Basic_format_arg_type _Type : 4; }; From 6fc4e4763293700086dac2e7e3515503ea69da10 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 17:01:22 +0100 Subject: [PATCH 12/36] No shadowing --- stl/inc/format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index fabcda6ec24..0748fae1f37 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -391,7 +391,7 @@ class basic_format_arg { public: using _CharType = typename _Context::char_type; - template + template friend auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)); class handle { From 6e46b0796996428c66ee49a7d01d0ca41f5a2359 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 18:34:19 +0100 Subject: [PATCH 13/36] Dont use friendship --- stl/inc/format | 4 ---- 1 file changed, 4 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 0748fae1f37..1adf987d739 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -391,9 +391,6 @@ class basic_format_arg { public: using _CharType = typename _Context::char_type; - template - friend auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)); - class handle { private: const void* _Ptr; @@ -451,7 +448,6 @@ public: return _Active_state != _Basic_format_arg_type::_None; } -private: _Basic_format_arg_type _Active_state = _Basic_format_arg_type::_None; union { monostate _No_state = monostate{}; From 2700e2a4b7960d111893e440b3c60539e2f087c7 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 18:35:03 +0100 Subject: [PATCH 14/36] Remove all int128 code --- stl/inc/format | 31 ++----------------------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 1adf987d739..d5d2c5544d6 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -372,8 +372,6 @@ enum class _Basic_format_arg_type : uint8_t { _UInt_type, _Long_type, _ULong_type, - //_Int128_type, - //_UInt128_type, _Bool_type, _Char_type, _Float_type, @@ -422,10 +420,6 @@ public: : _Active_state(_Basic_format_arg_type::_Long_type), _Long_state(_Val) {} explicit basic_format_arg(const unsigned long long _Val) noexcept : _Active_state(_Basic_format_arg_type::_ULong_type), _ULong_state(_Val) {} - // explicit basic_format_arg(const __int128 _Val) noexcept - // : _Active_state(_Basic_format_arg_type::_Int128_type), _Int128_state(_Val) {} - // explicit basic_format_arg(const __uint128 _Val) noexcept - // : _Active_state(_Basic_format_arg_type::_UInt128_type), _UInt128_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 @@ -455,8 +449,6 @@ public: unsigned _UInt_state; long long _Long_state; unsigned long long _ULong_state; - // __int128 _Int128_state; - // __uint128 _UInt128_state; bool _Bool_state; _CharType _Char_state; float _Float_state; @@ -482,10 +474,6 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt return _Vis(_Arg._Long_state); case _Basic_format_arg_type::_ULong_type: return _Vis(_Arg._ULong_state); - // case _Basic_format_arg_type::_Int128_type: - // return _Vis(_Arg._Int128_state); - // case _Basic_format_arg_type::_UInt128_type: - // return _Vis(_Arg._UInt128_state); case _Basic_format_arg_type::_Bool_type: return _Vis(_Arg._Bool_state); case _Basic_format_arg_type::_Char_type: @@ -523,10 +511,6 @@ static constexpr auto _Get_format_arg_storage_type() noexcept { return static_cast(42); } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { return static_cast(42); - //} else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(__int128)) { - // return __int128{42}; - //} else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(__unint128)) { - // return __uint128{42}; } else if constexpr (is_same_v<_Ty, bool>) { return true; } else if constexpr (is_same_v<_Ty, _CharType>) { @@ -658,13 +642,7 @@ public: } }; -// TRANSITION, Contraints missing, default _Context missing -template -_Format_arg_store<_Context> make_format_args() { - return _Format_arg_store<_Context>{}; -} - -template +template _Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; size_t _Arg_index = 0; @@ -672,8 +650,7 @@ _Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { return _Arg_store; } -// TRANSITION, Contraints missing, WContext missing -template +template _Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; size_t _Arg_index = 0; @@ -716,10 +693,6 @@ public: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_ULong_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - // case _Basic_format_arg_type::_Int128_type: - // return basic_format_arg<_Context>{_Get_value_from_memory<__int128>(_Arg_storage)}; - // case _Basic_format_arg_type::_UInt128_type: - // return basic_format_arg<_Context>{_Get_value_from_memory<__uint128>(_Arg_storage)}; case _Basic_format_arg_type::_Bool_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Char_type: From dd88cc0013a915dd60ca16a6c63d7bb767c3b69e Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 20:19:27 +0100 Subject: [PATCH 15/36] Fix some bugs --- stl/inc/format | 18 +++++++++++------- .../P0645R10_text_formatting_args/test.cpp | 12 ++++++------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index d5d2c5544d6..fd317b29e27 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -503,6 +503,8 @@ static constexpr auto _Get_format_arg_storage_type() noexcept { using _CharType = typename _Context::char_type; if constexpr (is_same_v<_Ty, monostate>) { return monostate{}; + } else if constexpr (is_same_v<_Ty, _CharType>) { + return _CharType{}; } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { return int{}; } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned)) { @@ -513,8 +515,6 @@ static constexpr auto _Get_format_arg_storage_type() noexcept { return static_cast(42); } else if constexpr (is_same_v<_Ty, bool>) { return true; - } else if constexpr (is_same_v<_Ty, _CharType>) { - return _CharType{}; } else if constexpr (is_same_v<_Ty, float>) { return float{}; } else if constexpr (is_same_v<_Ty, double>) { @@ -568,7 +568,7 @@ private: std::memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); _Index_array[_Arg_index]._Type = _Arg_type; - if (_Arg_index < _Num_args) { + if (_Arg_index + 1 < _Num_args) { // Set the starting index of the next arg, as that is dynamic _Index_array[_Arg_index + 1] = _Format_arg_store_packed_index{_Store_index + _Length}; } } @@ -645,16 +645,20 @@ public: template _Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); + if constexpr (sizeof...(_Args) > 0) { + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + } return _Arg_store; } template _Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); + if constexpr (sizeof...(_Args) > 0) { + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + } return _Arg_store; } diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 42895a64d6d..e475a704d0a 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -164,18 +164,18 @@ void test_format_arg_store() { if constexpr (is_same_v) { test_single_format_arg(42); - test_single_format_arg(42); + test_single_format_arg(42); } else { - test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); } - test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); #ifdef __cpp_char8_t - test_single_format_arg(42); + test_single_format_arg(42); #endif // __cpp_char8_t - test_single_format_arg(42); - test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); From 218ffc1768d6b1ae6e2869f0387e6fd0b98f5fc2 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 20:31:31 +0100 Subject: [PATCH 16/36] Who doesnt hate max/min --- stl/inc/format | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index fd317b29e27..2e6a175b6b4 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -573,8 +573,8 @@ private: } } - byte _Storage[(_STD max)(_Storage_length, 1ull)]; - _Index_type _Index_array[(_STD max)(_Num_args, 1ull)]; + byte _Storage[(_STD max)(_Storage_length, static_cast(1))]; + _Index_type _Index_array[(_STD max)(_Num_args, static_cast(1))]; public: // See [format.arg]/5 From abb61b190a476aae08068b575d7f3dd594b67514 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 5 Feb 2021 20:56:37 +0100 Subject: [PATCH 17/36] Copy&Paste does not like ODR --- stl/inc/format | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 2e6a175b6b4..a6beb1813c3 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -499,7 +499,7 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt } template -static constexpr auto _Get_format_arg_storage_type() noexcept { +constexpr auto _Get_format_arg_storage_type() noexcept { using _CharType = typename _Context::char_type; if constexpr (is_same_v<_Ty, monostate>) { return monostate{}; @@ -533,7 +533,7 @@ static constexpr auto _Get_format_arg_storage_type() noexcept { } template -static constexpr size_t _Get_format_arg_storage_size() noexcept { +constexpr size_t _Get_format_arg_storage_size() noexcept { return sizeof(_Get_format_arg_storage_type<_Context, _Ty>()); } From c23e34c7c3381795bd0eaa3385d548d6161e34ee Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 20:49:06 +0100 Subject: [PATCH 18/36] Address review comments Co-authored by: Casey Carter --- stl/inc/format | 101 +++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index a6beb1813c3..d52ae3ec7ee 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -333,11 +333,11 @@ public: // 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 == -1) { + if (_Next_arg_id < 0) { throw format_error("Can not switch from manual to automatic indexing"); } @@ -370,13 +370,13 @@ enum class _Basic_format_arg_type : uint8_t { _None, _Int_type, _UInt_type, - _Long_type, - _ULong_type, + _LLong_type, + _ULLong_type, _Bool_type, _Char_type, _Float_type, _Double_type, - _Long_double_type, + _LDouble_type, _Pointer_type, _CString_type, _String_type, @@ -400,9 +400,9 @@ public: 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> _Fn; - _Parse_ctx.advance_to(_Fn.parse(_Parse_ctx)); - _Format_ctx.advance_to(_Fn.format(*static_cast(_Ptr), _Format_ctx)); + 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) { @@ -410,16 +410,17 @@ public: } }; + // 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 _Val) noexcept + 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_type), _Long_state(_Val) {} - explicit basic_format_arg(const unsigned long long _Val) noexcept - : _Active_state(_Basic_format_arg_type::_ULong_type), _ULong_state(_Val) {} + : _Active_state(_Basic_format_arg_type::_LLong_type), _LLong_state(_Val) {} + explicit basic_format_arg(const unsigned int long long _Val) noexcept + : _Active_state(_Basic_format_arg_type::_ULLong_type), _ULLong_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 @@ -429,7 +430,7 @@ public: 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), _LDouble_state(_Val) {} + : _Active_state(_Basic_format_arg_type::_LDouble_type), _LDouble_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 @@ -446,9 +447,9 @@ public: union { monostate _No_state = monostate{}; int _Int_state; - unsigned _UInt_state; - long long _Long_state; - unsigned long long _ULong_state; + unsigned int _UInt_state; + long long _LLong_state; + unsigned int long long _ULLong_state; bool _Bool_state; _CharType _Char_state; float _Float_state; @@ -470,10 +471,10 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt return _Vis(_Arg._Int_state); case _Basic_format_arg_type::_UInt_type: return _Vis(_Arg._UInt_state); - case _Basic_format_arg_type::_Long_type: - return _Vis(_Arg._Long_state); - case _Basic_format_arg_type::_ULong_type: - return _Vis(_Arg._ULong_state); + case _Basic_format_arg_type::_LLong_type: + return _Vis(_Arg._LLong_state); + case _Basic_format_arg_type::_ULLong_type: + return _Vis(_Arg._ULLong_state); case _Basic_format_arg_type::_Bool_type: return _Vis(_Arg._Bool_state); case _Basic_format_arg_type::_Char_type: @@ -482,7 +483,7 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt 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: + case _Basic_format_arg_type::_LDouble_type: return _Vis(_Arg._LDouble_state); case _Basic_format_arg_type::_Pointer_type: return _Vis(_Arg._Pointer_state); @@ -499,22 +500,24 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt } template -constexpr auto _Get_format_arg_storage_type() noexcept { +/* consteval */ constexpr auto _Get_format_arg_storage_type() noexcept { using _CharType = typename _Context::char_type; if constexpr (is_same_v<_Ty, monostate>) { return monostate{}; } else if constexpr (is_same_v<_Ty, _CharType>) { return _CharType{}; + } else if constexpr (is_same_v<_Ty, char> && is_same_v<_CharType, wchar_t>) { + return _CharType{}; } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { return int{}; - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned)) { - return unsigned{}; + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int)) { + return unsigned int{}; } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { return static_cast(42); - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { - return static_cast(42); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int long long)) { + return static_cast(42); } else if constexpr (is_same_v<_Ty, bool>) { - return true; + return bool{}; } else if constexpr (is_same_v<_Ty, float>) { return float{}; } else if constexpr (is_same_v<_Ty, double>) { @@ -533,7 +536,7 @@ constexpr auto _Get_format_arg_storage_type() noexcept { } template -constexpr size_t _Get_format_arg_storage_size() noexcept { +/* consteval */ constexpr size_t _Get_format_arg_storage_size() noexcept { return sizeof(_Get_format_arg_storage_type<_Context, _Ty>()); } @@ -542,7 +545,7 @@ struct _Format_arg_store_packed_index { using _Index_type = size_t; constexpr _Format_arg_store_packed_index() = default; - constexpr _Format_arg_store_packed_index(const size_t _Index) + explicit constexpr _Format_arg_store_packed_index(const size_t _Index) : _Index(static_cast<_Index_type>(_Index)), _Type(_Basic_format_arg_type::_None) {} _Index_type _Index : (sizeof(_Index_type) * 8 - 4); @@ -552,8 +555,7 @@ struct _Format_arg_store_packed_index { template class _Format_arg_store { private: - template - friend class basic_format_args; + friend class basic_format_args<_Context>; static constexpr size_t _Num_args = sizeof...(_Args); static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args>() + ... + 0); @@ -566,7 +568,7 @@ private: const auto _Store_index = _Index_array[_Arg_index]._Index; const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>(); - std::memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); + _CSTD memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); _Index_array[_Arg_index]._Type = _Arg_type; if (_Arg_index + 1 < _Num_args) { // Set the starting index of the next arg, as that is dynamic _Index_array[_Arg_index + 1] = _Format_arg_store_packed_index{_Store_index + _Length}; @@ -580,21 +582,21 @@ public: // See [format.arg]/5 template void _Store(const size_t _Arg_index, const _Ty& _Val) noexcept { - if constexpr (_Same_impl<_Ty, bool>) { + if constexpr (is_same_v<_Ty, bool>) { _Store_impl(_Arg_index, _Val); - } else if constexpr (_Same_impl<_Ty, _CharType>) { + } else if constexpr (is_same_v<_Ty, _CharType>) { _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, _Val); - } else if constexpr (_Same_impl<_Ty, char> && _Same_impl<_CharType, wchar_t>) { + } else if constexpr (is_same_v<_Ty, char> && is_same_v<_CharType, wchar_t>) { _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, static_cast(_Val)); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { _Store_impl(_Arg_index, static_cast(_Val)); - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned)) { - _Store_impl(_Arg_index, static_cast(_Val)); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int)) { + _Store_impl(_Arg_index, static_cast(_Val)); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { - _Store_impl(_Arg_index, static_cast(_Val)); - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { - _Store_impl( - _Arg_index, static_cast(_Val)); + _Store_impl(_Arg_index, static_cast(_Val)); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int long long)) { + _Store_impl( + _Arg_index, static_cast(_Val)); } else { _Store_impl::handle, _Basic_format_arg_type::_Custom_type>( _Arg_index, basic_format_arg<_Context>::handle(_Val)); @@ -610,7 +612,7 @@ public: } void _Store(const size_t _Arg_index, long double _Val) noexcept { - _Store_impl(_Arg_index, _Val); + _Store_impl(_Arg_index, _Val); } void _Store(const size_t _Arg_index, const _CharType* _Val) noexcept { @@ -669,8 +671,7 @@ private: template _NODISCARD static constexpr auto _Get_value_from_memory(const byte* _Storage) noexcept { - byte _Temp[sizeof(_Ty)]; - _Copy_unchecked(_Storage, _Storage + sizeof(_Ty), _Temp); + auto& _Temp = *reinterpret_cast(_Storage); return _Bit_cast<_Ty>(_Temp); } @@ -692,11 +693,11 @@ public: case _Basic_format_arg_type::_Int_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_UInt_type: - return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_Long_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_LLong_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_ULong_type: - return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_ULLong_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Bool_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Char_type: @@ -705,7 +706,7 @@ public: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Double_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_Long_double_type: + case _Basic_format_arg_type::_LDouble_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Pointer_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; From bed93292a3863740895fda81da20324fe75f3343 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 21:41:37 +0100 Subject: [PATCH 19/36] Minor fixes --- stl/inc/format | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 6fe500f4b87..ba97aaec5fe 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -511,7 +511,7 @@ template } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { return int{}; } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int)) { - return unsigned int{}; + return static_cast(42); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { return static_cast(42); } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int long long)) { @@ -555,7 +555,8 @@ struct _Format_arg_store_packed_index { template class _Format_arg_store { private: - friend class basic_format_args<_Context>; + template + friend class basic_format_args; static constexpr size_t _Num_args = sizeof...(_Args); static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args>() + ... + 0); From a4a8be6229de5851803821bc95e5d1de0d792a4a Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 21:55:37 +0100 Subject: [PATCH 20/36] Move make_format_args down --- stl/inc/format | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index ba97aaec5fe..1ee8dfdd4b4 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -645,26 +645,6 @@ public: } }; -template -_Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { - _Format_arg_store<_Context, _Args...> _Arg_store{}; - if constexpr (sizeof...(_Args) > 0) { - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); - } - return _Arg_store; -} - -template -_Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { - _Format_arg_store<_Context, _Args...> _Arg_store{}; - if constexpr (sizeof...(_Args) > 0) { - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); - } - return _Arg_store; -} - template class basic_format_args { private: @@ -769,6 +749,26 @@ using wformat_context = basic_format_context, wstr using format_args = basic_format_args; using wformat_args = basic_format_args; +template +_Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { + _Format_arg_store<_Context, _Args...> _Arg_store{}; + if constexpr (sizeof...(_Args) > 0) { + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + } + return _Arg_store; +} + +template +_Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { + _Format_arg_store<_Context, _Args...> _Arg_store{}; + if constexpr (sizeof...(_Args) > 0) { + size_t _Arg_index = 0; + (_Arg_store._Store(_Arg_index++, args), ...); + } + return _Arg_store; +} + // FUNCTION vformat string vformat(string_view _Fmt, format_args _Args); wstring vformat(wstring_view _Fmt, wformat_args _Args); From 71bf674a61c110c5dc42edb609aa509f254e61e4 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 21:56:28 +0100 Subject: [PATCH 21/36] Use auto --- stl/inc/format | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 1ee8dfdd4b4..60eb17c7fe2 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -750,7 +750,7 @@ using format_args = basic_format_args; using wformat_args = basic_format_args; template -_Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { +auto make_format_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; if constexpr (sizeof...(_Args) > 0) { size_t _Arg_index = 0; @@ -760,7 +760,7 @@ _Format_arg_store<_Context, _Args...> make_format_args(const _Args&... args) { } template -_Format_arg_store<_Context, _Args...> make_wformat_args(const _Args&... args) { +auto make_wformat_args(const _Args&... args) { _Format_arg_store<_Context, _Args...> _Arg_store{}; if constexpr (sizeof...(_Args) > 0) { size_t _Arg_index = 0; From 28434176fb883547159c70303a84a4d1cdb89a32 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 22:28:44 +0100 Subject: [PATCH 22/36] Some more fixes --- .../P0645R10_text_formatting_args/test.cpp | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index e475a704d0a..69bb23289ff 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -12,10 +12,6 @@ using namespace std; -#pragma warning(push) -#pragma warning(disable : 4582) // 'uninit_vector::storage': constructor is not implicitly called -#pragma warning(disable : 4583) // 'uninit_vector::storage': destructor is not implicitly called - template constexpr auto get_input_literal() { if constexpr (same_as) { @@ -69,7 +65,7 @@ auto visitor = [](auto&& arg) { return Arg_type::none; } else if constexpr (is_same_v) { return Arg_type::int_type; - } else if constexpr (is_same_v) { + } else if constexpr (is_same_v) { return Arg_type::unsigned_type; } else if constexpr (is_same_v) { return Arg_type::long_type; @@ -190,9 +186,13 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); - test_single_format_arg(42); + if constexpr (sizeof(int) == sizeof(ptrdiff_t)) { + test_single_format_arg(42); + } else { + test_single_format_arg(42); + } - test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); @@ -205,7 +205,11 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); - test_single_format_arg(42); + if constexpr (sizeof(unsigned int) == sizeof(size_t)) { + test_single_format_arg(42); + } else { + test_single_format_arg(42); + } test_single_format_arg(42.f); test_single_format_arg(42.); @@ -223,4 +227,3 @@ int main() { test_format_arg_store(); test_format_arg_store(); } -#pragma warning(pop) From 0f4b962f36e33e6c14ab9b47a0809e63db8cb438 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 23:00:46 +0100 Subject: [PATCH 23/36] Everyone can be a wchar_t if he just believe in it --- .../std/tests/P0645R10_text_formatting_args/test.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 69bb23289ff..2a7f10d86cd 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -196,9 +196,17 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); - test_single_format_arg(42); + if constexpr (is_same_v) { + test_single_format_arg(42); + } else { + test_single_format_arg(42); + } test_single_format_arg(42); - test_single_format_arg(42); + if constexpr (is_same_v) { + test_single_format_arg(42); + } else { + test_single_format_arg(42); + } test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); From ef8eea4df14d005b39d1665f8d1f4509724c6e3e Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 6 Feb 2021 23:07:27 +0100 Subject: [PATCH 24/36] Add {unsigned} long tests --- tests/std/tests/P0645R10_text_formatting_args/test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 2a7f10d86cd..40f61f5ab38 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -174,6 +174,7 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); @@ -193,6 +194,7 @@ void test_format_arg_store() { } test_single_format_arg(42); + test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); From d50f51d32919dc846e6138e36493c817c443a90a Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 7 Feb 2021 09:48:08 +0100 Subject: [PATCH 25/36] Int is in our hearts --- tests/std/tests/P0645R10_text_formatting_args/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 40f61f5ab38..826ba259ffc 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -134,7 +134,7 @@ void test_basic_format_arg() { } } template -void test_empty_format_arg_int() { +void test_empty_format_arg() { const auto store = make_format_args(); const basic_format_args args{store}; const same_as> auto first_arg = args.get(0); @@ -156,7 +156,7 @@ template void test_format_arg_store() { using char_type = typename Context::char_type; - test_empty_format_arg_int(); + test_empty_format_arg(); if constexpr (is_same_v) { test_single_format_arg(42); From a152b998eb6a2c70675f1984f327f52a853de810 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 7 Feb 2021 09:53:36 +0100 Subject: [PATCH 26/36] Remove duplication --- tests/std/tests/P0645R10_text_formatting_args/test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index 826ba259ffc..ac5a11b7bf3 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -158,11 +158,10 @@ void test_format_arg_store() { test_empty_format_arg(); + test_single_format_arg(42); if constexpr (is_same_v) { - test_single_format_arg(42); test_single_format_arg(42); } else { - test_single_format_arg(42); test_single_format_arg(42); } test_single_format_arg(42); From 87b4851306fa8f4820f9dff7b14c65795555a96d Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Mon, 8 Feb 2021 08:19:47 +0100 Subject: [PATCH 27/36] Move _CharType alias into function to unblock the build --- stl/inc/format | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 60eb17c7fe2..be7ac3c8e26 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -648,8 +648,6 @@ public: template class basic_format_args { private: - using _CharType = typename _Context::char_type; - template _NODISCARD static constexpr auto _Get_value_from_memory(const byte* _Storage) noexcept { auto& _Temp = *reinterpret_cast(_Storage); @@ -667,6 +665,7 @@ public: return basic_format_arg<_Context>{}; } + using _CharType = typename _Context::char_type; const byte* _Arg_storage = _Storage + _Index_array[_Index]._Index; switch (_Index_array[_Index]._Type) { case _Basic_format_arg_type::_None: From 3caf20e441b80f7d53d46a40a54ebc033d688999 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 10 Feb 2021 21:24:42 +0100 Subject: [PATCH 28/36] First round of review comments --- stl/inc/format | 140 +++++++++--------- .../P0645R10_text_formatting_args/test.cpp | 27 ++-- 2 files changed, 86 insertions(+), 81 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index be7ac3c8e26..36a283880b5 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -370,13 +370,13 @@ enum class _Basic_format_arg_type : uint8_t { _None, _Int_type, _UInt_type, - _LLong_type, - _ULLong_type, + _Long_long_type, + _ULong_long_type, _Bool_type, _Char_type, _Float_type, _Double_type, - _LDouble_type, + _Long_double_type, _Pointer_type, _CString_type, _String_type, @@ -403,7 +403,7 @@ public: 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); @@ -418,9 +418,9 @@ public: 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::_LLong_type), _LLong_state(_Val) {} - explicit basic_format_arg(const unsigned int long long _Val) noexcept - : _Active_state(_Basic_format_arg_type::_ULLong_type), _ULLong_state(_Val) {} + : _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 @@ -430,7 +430,7 @@ public: 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::_LDouble_type), _LDouble_state(_Val) {} + : _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 @@ -448,13 +448,13 @@ public: monostate _No_state = monostate{}; int _Int_state; unsigned int _UInt_state; - long long _LLong_state; - unsigned int long long _ULLong_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 _LDouble_state; + long double _Long_double_state; const void* _Pointer_state; const _CharType* _CString_state; basic_string_view<_CharType> _String_state; @@ -463,7 +463,7 @@ public: }; template -auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> decltype(_Vis(0)) { +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); @@ -471,10 +471,10 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt return _Vis(_Arg._Int_state); case _Basic_format_arg_type::_UInt_type: return _Vis(_Arg._UInt_state); - case _Basic_format_arg_type::_LLong_type: - return _Vis(_Arg._LLong_state); - case _Basic_format_arg_type::_ULLong_type: - return _Vis(_Arg._ULLong_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: @@ -483,8 +483,8 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt return _Vis(_Arg._Float_state); case _Basic_format_arg_type::_Double_type: return _Vis(_Arg._Double_state); - case _Basic_format_arg_type::_LDouble_type: - return _Vis(_Arg._LDouble_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: @@ -494,7 +494,7 @@ auto visit_format_arg(_Visitor&& _Vis, basic_format_arg<_Context> _Arg) -> declt case _Basic_format_arg_type::_Custom_type: return _Vis(_Arg._Custom_state); default: - _STL_ASSERT(false, "Invalid basic_format_arg type"); + _STL_VERIFY(false, "basic_format_arg is in impossible state"); return _Vis(0); } } @@ -514,8 +514,8 @@ template return static_cast(42); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { return static_cast(42); - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int long long)) { - return static_cast(42); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { + return static_cast(42); } else if constexpr (is_same_v<_Ty, bool>) { return bool{}; } else if constexpr (is_same_v<_Ty, float>) { @@ -545,18 +545,20 @@ struct _Format_arg_store_packed_index { using _Index_type = size_t; constexpr _Format_arg_store_packed_index() = default; - explicit constexpr _Format_arg_store_packed_index(const size_t _Index) + constexpr explicit _Format_arg_store_packed_index(const size_t _Index) : _Index(static_cast<_Index_type>(_Index)), _Type(_Basic_format_arg_type::_None) {} _Index_type _Index : (sizeof(_Index_type) * 8 - 4); _Basic_format_arg_type _Type : 4; }; +template +class basic_format_args; + template class _Format_arg_store { private: - template - friend class basic_format_args; + friend basic_format_args<_Context>; static constexpr size_t _Num_args = sizeof...(_Args); static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args>() + ... + 0); @@ -564,8 +566,8 @@ private: using _CharType = typename _Context::char_type; using _Index_type = _Format_arg_store_packed_index; - template - void _Store_impl(const size_t _Arg_index, _Ty _Val) noexcept { + template + void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { const auto _Store_index = _Index_array[_Arg_index]._Index; const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>(); @@ -576,64 +578,66 @@ private: } } - byte _Storage[(_STD max)(_Storage_length, static_cast(1))]; + unsigned char _Storage[(_STD max)(_Storage_length, static_cast(1))]; _Index_type _Index_array[(_STD max)(_Num_args, static_cast(1))]; public: // See [format.arg]/5 template void _Store(const size_t _Arg_index, const _Ty& _Val) noexcept { + _Store_impl::handle>( + _Arg_index, _Basic_format_arg_type::_Custom_type, basic_format_arg<_Context>::handle(_Val)); + } + + // clang-format off + template + requires integral<_Ty> || floating_point<_Ty> + void _Store(const size_t _Arg_index, _Ty _Val) noexcept { + // clang-format on if constexpr (is_same_v<_Ty, bool>) { - _Store_impl(_Arg_index, _Val); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Bool_type, _Val); } else if constexpr (is_same_v<_Ty, _CharType>) { - _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, _Val); + _Store_impl<_CharType>(_Arg_index, _Basic_format_arg_type::_Char_type, _Val); } else if constexpr (is_same_v<_Ty, char> && is_same_v<_CharType, wchar_t>) { - _Store_impl<_CharType, _Basic_format_arg_type::_Char_type>(_Arg_index, static_cast(_Val)); + _Store_impl<_CharType>(_Arg_index, _Basic_format_arg_type::_Char_type, static_cast(_Val)); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(int)) { - _Store_impl(_Arg_index, static_cast(_Val)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Int_type, static_cast(_Val)); } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int)) { - _Store_impl(_Arg_index, static_cast(_Val)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_UInt_type, static_cast(_Val)); } else if constexpr (signed_integral<_Ty> && sizeof(_Ty) <= sizeof(long long)) { - _Store_impl(_Arg_index, static_cast(_Val)); - } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned int long long)) { - _Store_impl( - _Arg_index, static_cast(_Val)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Long_long_type, static_cast(_Val)); + } else if constexpr (unsigned_integral<_Ty> && sizeof(_Ty) <= sizeof(unsigned long long)) { + _Store_impl( + _Arg_index, _Basic_format_arg_type::_ULong_long_type, static_cast(_Val)); + } else if constexpr (is_same_v<_Ty, float>) { + _Store_impl(_Arg_index, _Basic_format_arg_type::_Float_type, _Val); + } else if constexpr (is_same_v<_Ty, double>) { + _Store_impl(_Arg_index, _Basic_format_arg_type::_Double_type, _Val); + } else if constexpr (is_same_v<_Ty, long double>) { + _Store_impl(_Arg_index, _Basic_format_arg_type::_Long_double_type, _Val); } else { - _Store_impl::handle, _Basic_format_arg_type::_Custom_type>( - _Arg_index, basic_format_arg<_Context>::handle(_Val)); + _STL_VERIFY(false, "Invalid type passed to _Format_arg_store::_Store"); } } - void _Store(const size_t _Arg_index, float _Val) noexcept { - _Store_impl(_Arg_index, _Val); - } - - void _Store(const size_t _Arg_index, double _Val) noexcept { - _Store_impl(_Arg_index, _Val); - } - - void _Store(const size_t _Arg_index, long double _Val) noexcept { - _Store_impl(_Arg_index, _Val); - } - void _Store(const size_t _Arg_index, const _CharType* _Val) noexcept { - _Store_impl(_Arg_index, _Val); + _Store_impl(_Arg_index, _Basic_format_arg_type::_CString_type, _Val); } template void _Store(const size_t _Arg_index, basic_string_view<_CharType, _Traits> _Val) noexcept { - _Store_impl, _Basic_format_arg_type::_String_type>( - _Arg_index, basic_string_view<_CharType>{_Val}); + _Store_impl>( + _Arg_index, _Basic_format_arg_type::_String_type, basic_string_view<_CharType>{_Val}); } template - void _Store(const size_t _Arg_index, basic_string<_CharType, _Traits, _Alloc> _Val) noexcept { - _Store_impl, _Basic_format_arg_type::_String_type>( + void _Store(const size_t _Arg_index, const basic_string<_CharType, _Traits, _Alloc>& _Val) noexcept { + _Store_impl>( _Arg_index, _Basic_format_arg_type::_String_type, basic_string_view<_CharType>{_Val.data(), _Val.size()}); } void _Store(const size_t _Arg_index, nullptr_t) noexcept { - _Store_impl(_Arg_index, static_cast(nullptr)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Pointer_type, static_cast(nullptr)); } // clang-format off @@ -641,7 +645,7 @@ public: requires is_void_v<_Ty> void _Store(const size_t _Arg_index, const _Ty* p) noexcept { // clang-format on - _Store_impl(_Arg_index, static_cast(p)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Pointer_type, static_cast(p)); } }; @@ -649,24 +653,24 @@ template class basic_format_args { private: template - _NODISCARD static constexpr auto _Get_value_from_memory(const byte* _Storage) noexcept { - auto& _Temp = *reinterpret_cast(_Storage); + _NODISCARD static auto _Get_value_from_memory(const unsigned char* _Storage) noexcept { + auto& _Temp = *reinterpret_cast(_Storage); return _Bit_cast<_Ty>(_Temp); } public: basic_format_args() noexcept; template - basic_format_args(const _Format_arg_store<_Context, _Args...>& store) noexcept - : _Num_args(sizeof...(_Args)), _Storage(store._Storage), _Index_array(store._Index_array) {} + basic_format_args(const _Format_arg_store<_Context, _Args...>& _Store) noexcept + : _Num_args(sizeof...(_Args)), _Storage(_Store._Storage), _Index_array(_Store._Index_array) {} basic_format_arg<_Context> get(const size_t _Index) const noexcept { if (_Index >= _Num_args) { return basic_format_arg<_Context>{}; } - using _CharType = typename _Context::char_type; - const byte* _Arg_storage = _Storage + _Index_array[_Index]._Index; + using _CharType = typename _Context::char_type; + const unsigned char* _Arg_storage = _Storage + _Index_array[_Index]._Index; switch (_Index_array[_Index]._Type) { case _Basic_format_arg_type::_None: return basic_format_arg<_Context>{}; @@ -674,10 +678,10 @@ public: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_UInt_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_LLong_type: + case _Basic_format_arg_type::_Long_long_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_ULLong_type: - return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; + case _Basic_format_arg_type::_ULong_long_type: + return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Bool_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Char_type: @@ -686,7 +690,7 @@ public: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Double_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; - case _Basic_format_arg_type::_LDouble_type: + case _Basic_format_arg_type::_Long_double_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; case _Basic_format_arg_type::_Pointer_type: return basic_format_arg<_Context>{_Get_value_from_memory(_Arg_storage)}; @@ -705,7 +709,7 @@ public: private: size_t _Num_args = 0; - const byte* _Storage = nullptr; + const unsigned char* _Storage = nullptr; const _Format_arg_store_packed_index* _Index_array = nullptr; }; diff --git a/tests/std/tests/P0645R10_text_formatting_args/test.cpp b/tests/std/tests/P0645R10_text_formatting_args/test.cpp index ac5a11b7bf3..08355b8a90f 100644 --- a/tests/std/tests/P0645R10_text_formatting_args/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_args/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -44,8 +45,8 @@ enum class Arg_type : uint8_t { none, int_type, unsigned_type, - long_type, - unsigned_long_type, + long_long_type, + unsigned_long_long_type, bool_type, char_type, float_type, @@ -68,9 +69,9 @@ auto visitor = [](auto&& arg) { } else if constexpr (is_same_v) { return Arg_type::unsigned_type; } else if constexpr (is_same_v) { - return Arg_type::long_type; + return Arg_type::long_long_type; } else if constexpr (is_same_v) { - return Arg_type::unsigned_long_type; + return Arg_type::unsigned_long_long_type; } else if constexpr (is_same_v) { return Arg_type::char_type; } else if constexpr (is_same_v) { @@ -116,7 +117,7 @@ void test_basic_format_arg() { basic_format_arg from_double{5.0}; assert(from_double); - basic_format_arg from_long_double{static_cast(5.0)}; + basic_format_arg from_long_double{5.0L}; assert(from_long_double); basic_format_arg from_pointer{static_cast(nullptr)}; @@ -183,13 +184,13 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); - test_single_format_arg(42); - test_single_format_arg(42); - test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); if constexpr (sizeof(int) == sizeof(ptrdiff_t)) { test_single_format_arg(42); } else { - test_single_format_arg(42); + test_single_format_arg(42); } test_single_format_arg(42); @@ -211,13 +212,13 @@ void test_format_arg_store() { test_single_format_arg(42); test_single_format_arg(42); test_single_format_arg(42); - test_single_format_arg(42); - test_single_format_arg(42); - test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); + test_single_format_arg(42); if constexpr (sizeof(unsigned int) == sizeof(size_t)) { test_single_format_arg(42); } else { - test_single_format_arg(42); + test_single_format_arg(42); } test_single_format_arg(42.f); From c5c8bbcc71bb12e7431cfde2ff5fa94dd0e6f926 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 10 Feb 2021 21:50:22 +0100 Subject: [PATCH 29/36] Use variable template rater than function --- stl/inc/format | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 36a283880b5..439ee0ce4cb 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -536,9 +536,7 @@ template } template -/* consteval */ constexpr size_t _Get_format_arg_storage_size() noexcept { - return sizeof(_Get_format_arg_storage_type<_Context, _Ty>()); -} +inline constexpr size_t _Get_format_arg_storage_size = sizeof(_Get_format_arg_storage_type<_Context, _Ty>()); struct _Format_arg_store_packed_index { // TRANSITION, Should be templated on number of arguments for even less storage @@ -561,7 +559,7 @@ private: friend basic_format_args<_Context>; static constexpr size_t _Num_args = sizeof...(_Args); - static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args>() + ... + 0); + static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args> + ... + 0); using _CharType = typename _Context::char_type; using _Index_type = _Format_arg_store_packed_index; @@ -569,7 +567,7 @@ private: template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { const auto _Store_index = _Index_array[_Arg_index]._Index; - const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>(); + const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>; _CSTD memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); _Index_array[_Arg_index]._Type = _Arg_type; From f3c91d01cc4b0f629dee261497e027d1f8d0e1b3 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 10 Feb 2021 21:57:19 +0100 Subject: [PATCH 30/36] Add warning --- stl/inc/format | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index 439ee0ce4cb..92b9d193ba2 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -571,7 +571,8 @@ private: _CSTD memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); _Index_array[_Arg_index]._Type = _Arg_type; - if (_Arg_index + 1 < _Num_args) { // Set the starting index of the next arg, as that is dynamic + if (_Arg_index + 1 < _Num_args) { + // Set the starting index of the next arg, as that is dynamic, must be called with increasing index _Index_array[_Arg_index + 1] = _Format_arg_store_packed_index{_Store_index + _Length}; } } From 8afe67b6f52a9e8d0299b597aee84b82dc81f8d7 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 11 Feb 2021 21:01:54 +0100 Subject: [PATCH 31/36] Make a proper constructor of _Format_arg_store --- stl/inc/format | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 92b9d193ba2..fa98c9ed75a 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -564,6 +564,9 @@ private: using _CharType = typename _Context::char_type; using _Index_type = _Format_arg_store_packed_index; + unsigned char _Storage[(_STD max)(_Storage_length, static_cast(1))]; + _Index_type _Index_array[(_STD max)(_Num_args, static_cast(1))]; + template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { const auto _Store_index = _Index_array[_Arg_index]._Index; @@ -577,10 +580,6 @@ private: } } - unsigned char _Storage[(_STD max)(_Storage_length, static_cast(1))]; - _Index_type _Index_array[(_STD max)(_Num_args, static_cast(1))]; - -public: // See [format.arg]/5 template void _Store(const size_t _Arg_index, const _Ty& _Val) noexcept { @@ -646,6 +645,14 @@ public: // clang-format on _Store_impl(_Arg_index, _Basic_format_arg_type::_Pointer_type, static_cast(p)); } + +public: + _Format_arg_store(const _Args&... args) noexcept { + if constexpr (sizeof...(_Args) > 0) { + size_t _Arg_index = 0; + (_Store(_Arg_index++, args), ...); + } + } }; template @@ -753,22 +760,12 @@ using wformat_args = basic_format_args; template auto make_format_args(const _Args&... args) { - _Format_arg_store<_Context, _Args...> _Arg_store{}; - if constexpr (sizeof...(_Args) > 0) { - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); - } - return _Arg_store; + return _Format_arg_store<_Context, _Args...>{args...}; } template auto make_wformat_args(const _Args&... args) { - _Format_arg_store<_Context, _Args...> _Arg_store{}; - if constexpr (sizeof...(_Args) > 0) { - size_t _Arg_index = 0; - (_Arg_store._Store(_Arg_index++, args), ...); - } - return _Arg_store; + return _Format_arg_store<_Context, _Args...>{args...}; } // FUNCTION vformat From 06cb9e9ff5305b41591beb038b74aaeddfa320c0 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 11 Feb 2021 21:57:54 +0100 Subject: [PATCH 32/36] Use a single contiguous array to store the `_Format_arg_store` data --- stl/inc/format | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index fa98c9ed75a..b16b7015c8d 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -556,23 +556,25 @@ class basic_format_args; template class _Format_arg_store { private: + using _CharType = typename _Context::char_type; + using _Index_type = _Format_arg_store_packed_index; + friend basic_format_args<_Context>; static constexpr size_t _Num_args = sizeof...(_Args); + static constexpr size_t _Index_length = _Num_args * sizeof(_Index_type); static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args> + ... + 0); - using _CharType = typename _Context::char_type; - using _Index_type = _Format_arg_store_packed_index; - - unsigned char _Storage[(_STD max)(_Storage_length, static_cast(1))]; - _Index_type _Index_array[(_STD max)(_Num_args, static_cast(1))]; + // we store the data in memory as _Format_arg_store_packed_index[_Index_length] + unsigned char[_Storage_length] + unsigned char _Storage[(_STD max)(_Index_length + _Storage_length, static_cast(1))]; template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { + const auto _Index_array = reinterpret_cast<_Index_type*>(_Storage); const auto _Store_index = _Index_array[_Arg_index]._Index; const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>; - _CSTD memcpy(_Storage + _Store_index, _STD addressof(_Val), _Length); + _CSTD memcpy(_Storage + _Index_length + _Store_index, _STD addressof(_Val), _Length); _Index_array[_Arg_index]._Type = _Arg_type; if (_Arg_index + 1 < _Num_args) { // Set the starting index of the next arg, as that is dynamic, must be called with increasing index @@ -659,8 +661,8 @@ template class basic_format_args { private: template - _NODISCARD static auto _Get_value_from_memory(const unsigned char* _Storage) noexcept { - auto& _Temp = *reinterpret_cast(_Storage); + _NODISCARD static auto _Get_value_from_memory(const unsigned char* _Val) noexcept { + auto& _Temp = *reinterpret_cast(_Val); return _Bit_cast<_Ty>(_Temp); } @@ -668,7 +670,7 @@ public: basic_format_args() noexcept; template basic_format_args(const _Format_arg_store<_Context, _Args...>& _Store) noexcept - : _Num_args(sizeof...(_Args)), _Storage(_Store._Storage), _Index_array(_Store._Index_array) {} + : _Num_args(sizeof...(_Args)), _Storage(_Store._Storage) {} basic_format_arg<_Context> get(const size_t _Index) const noexcept { if (_Index >= _Num_args) { @@ -676,8 +678,11 @@ public: } using _CharType = typename _Context::char_type; - const unsigned char* _Arg_storage = _Storage + _Index_array[_Index]._Index; - switch (_Index_array[_Index]._Type) { + const auto _Packed_index = reinterpret_cast(_Storage)[_Index]; + const auto _Index_length = _Num_args * sizeof(_Format_arg_store_packed_index); + const unsigned char* _Arg_storage = _Storage + _Index_length + _Packed_index._Index; + + switch (_Packed_index._Type) { case _Basic_format_arg_type::_None: return basic_format_arg<_Context>{}; case _Basic_format_arg_type::_Int_type: @@ -714,9 +719,8 @@ public: } private: - size_t _Num_args = 0; - const unsigned char* _Storage = nullptr; - const _Format_arg_store_packed_index* _Index_array = nullptr; + size_t _Num_args = 0; + const unsigned char* _Storage = nullptr; }; // TODO: test coverage From 302ac20b11c3d360c82bbe18798a7f16dbfb5e00 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 11 Feb 2021 22:09:47 +0100 Subject: [PATCH 33/36] Specialize empty `_Format_arg_store` --- stl/inc/format | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index b16b7015c8d..50836583b75 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -566,7 +566,7 @@ private: static constexpr size_t _Storage_length = (_Get_format_arg_storage_size<_Context, _Args> + ... + 0); // we store the data in memory as _Format_arg_store_packed_index[_Index_length] + unsigned char[_Storage_length] - unsigned char _Storage[(_STD max)(_Index_length + _Storage_length, static_cast(1))]; + unsigned char _Storage[_Index_length + _Storage_length]; template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { @@ -650,13 +650,14 @@ private: public: _Format_arg_store(const _Args&... args) noexcept { - if constexpr (sizeof...(_Args) > 0) { - size_t _Arg_index = 0; - (_Store(_Arg_index++, args), ...); - } + size_t _Arg_index = 0; + (_Store(_Arg_index++, args), ...); } }; +template +class _Format_arg_store<_Context> {}; + template class basic_format_args { private: @@ -668,6 +669,7 @@ private: public: basic_format_args() noexcept; + basic_format_args(const _Format_arg_store<_Context>&) noexcept {} template basic_format_args(const _Format_arg_store<_Context, _Args...>& _Store) noexcept : _Num_args(sizeof...(_Args)), _Storage(_Store._Storage) {} From 9f85da5531acdf6b6cf95b6ea222692a7236d4da Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 12 Feb 2021 08:03:17 +0100 Subject: [PATCH 34/36] More review comments --- stl/inc/format | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 50836583b75..dd7c6b1eb3c 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -616,7 +616,7 @@ private: } else if constexpr (is_same_v<_Ty, long double>) { _Store_impl(_Arg_index, _Basic_format_arg_type::_Long_double_type, _Val); } else { - _STL_VERIFY(false, "Invalid type passed to _Format_arg_store::_Store"); + static_assert(_Always_false<_Ty>, "Invalid type passed to _Format_arg_store::_Store"); } } @@ -643,15 +643,15 @@ private: // clang-format off template requires is_void_v<_Ty> - void _Store(const size_t _Arg_index, const _Ty* p) noexcept { + void _Store(const size_t _Arg_index, const _Ty* _Ptr) noexcept { // clang-format on - _Store_impl(_Arg_index, _Basic_format_arg_type::_Pointer_type, static_cast(p)); + _Store_impl(_Arg_index, _Basic_format_arg_type::_Pointer_type, static_cast(_Ptr)); } public: - _Format_arg_store(const _Args&... args) noexcept { + _Format_arg_store(const _Args&... _Vals) noexcept { size_t _Arg_index = 0; - (_Store(_Arg_index++, args), ...); + (_Store(_Arg_index++, _Vals), ...); } }; @@ -765,13 +765,13 @@ using format_args = basic_format_args; using wformat_args = basic_format_args; template -auto make_format_args(const _Args&... args) { - return _Format_arg_store<_Context, _Args...>{args...}; +auto make_format_args(const _Args&... _Vals) { + return _Format_arg_store<_Context, _Args...>{_Vals...}; } template -auto make_wformat_args(const _Args&... args) { - return _Format_arg_store<_Context, _Args...>{args...}; +auto make_wformat_args(const _Args&... _Vals) { + return _Format_arg_store<_Context, _Args...>{_Vals...}; } // FUNCTION vformat From 70fc37baf912bebf34f78c91020e032e32f2d8e8 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 12 Feb 2021 08:06:32 +0100 Subject: [PATCH 35/36] Fix uninitialized memory --- stl/inc/format | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index dd7c6b1eb3c..e40b417c442 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -570,8 +570,9 @@ private: template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { + // Note: _Index_array is uninitialized storage when we call it first with _Arg_index == 0 const auto _Index_array = reinterpret_cast<_Index_type*>(_Storage); - const auto _Store_index = _Index_array[_Arg_index]._Index; + const auto _Store_index = _Arg_index == 0 ? size_t{0} : _Index_array[_Arg_index]._Index; const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>; _CSTD memcpy(_Storage + _Index_length + _Store_index, _STD addressof(_Val), _Length); From 76b8a55255ff4dfec59a13e33ab9ff495fe02d32 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 12 Feb 2021 08:54:54 +0100 Subject: [PATCH 36/36] Properly initialize the first Index --- stl/inc/format | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index e40b417c442..83631ab8089 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -570,9 +570,8 @@ private: template void _Store_impl(const size_t _Arg_index, const _Basic_format_arg_type _Arg_type, _Ty _Val) noexcept { - // Note: _Index_array is uninitialized storage when we call it first with _Arg_index == 0 const auto _Index_array = reinterpret_cast<_Index_type*>(_Storage); - const auto _Store_index = _Arg_index == 0 ? size_t{0} : _Index_array[_Arg_index]._Index; + const auto _Store_index = _Index_array[_Arg_index]._Index; const auto _Length = _Get_format_arg_storage_size<_Context, _Ty>; _CSTD memcpy(_Storage + _Index_length + _Store_index, _STD addressof(_Val), _Length); @@ -651,6 +650,8 @@ private: public: _Format_arg_store(const _Args&... _Vals) noexcept { + // Note: _Storage is uninitialized, so manually initialize the first index + _STD construct_at(reinterpret_cast<_Format_arg_store_packed_index*>(_Storage)); size_t _Arg_index = 0; (_Store(_Arg_index++, _Vals), ...); }