From 1e9d92f016dad2debdfd89ad14deec0a46cb7558 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Sun, 30 Jul 2023 23:27:55 +0200 Subject: [PATCH 1/3] Implement debug-enabled `formatter` specializations * This PR implements debug-enabled standard `formatter` specializations, * Drive-by: rename tests added in #3656 - use `text_formatting_` prefix instead of `formatting_ranges_` (for consistency), * Towards #2919. --- stl/inc/format | 18 ++ tests/std/include/test_format_support.hpp | 16 ++ tests/std/test.lst | 7 +- .../env.lst | 0 .../test.cpp | 205 ++++++++++++++++++ .../P2286R8_text_formatting_escaping/env.lst | 4 + .../test.cpp | 0 .../env.lst | 0 .../test.cpp | 0 .../env.lst | 0 .../test.cpp | 0 11 files changed, 247 insertions(+), 3 deletions(-) rename tests/std/tests/{P2286R8_formatting_ranges => P2286R8_text_formatting_debug_enabled_specializations}/env.lst (100%) create mode 100644 tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp create mode 100644 tests/std/tests/P2286R8_text_formatting_escaping/env.lst rename tests/std/tests/{P2286R8_formatting_ranges => P2286R8_text_formatting_escaping}/test.cpp (100%) rename tests/std/tests/{P2286R8_formatting_ranges_legacy_text_encoding => P2286R8_text_formatting_escaping_legacy_text_encoding}/env.lst (100%) rename tests/std/tests/{P2286R8_formatting_ranges_legacy_text_encoding => P2286R8_text_formatting_escaping_legacy_text_encoding}/test.cpp (100%) rename tests/std/tests/{P2286R8_formatting_ranges_utf8 => P2286R8_text_formatting_escaping_utf8}/env.lst (100%) rename tests/std/tests/{P2286R8_formatting_ranges_utf8 => P2286R8_text_formatting_escaping_utf8}/test.cpp (100%) diff --git a/stl/inc/format b/stl/inc/format index 28804aa364a..44f0466d0c8 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -125,10 +125,18 @@ static_assert(static_cast(_Basic_format_arg_type::_Custom_type) < 16, "must _NODISCARD constexpr bool _Is_integral_fmt_type(_Basic_format_arg_type _Ty) { return _Ty > _Basic_format_arg_type::_None && _Ty <= _Basic_format_arg_type::_Char_type; } + _NODISCARD constexpr bool _Is_arithmetic_fmt_type(_Basic_format_arg_type _Ty) { return _Ty > _Basic_format_arg_type::_None && _Ty <= _Basic_format_arg_type::_Long_double_type; } +#if _HAS_CXX23 +_NODISCARD consteval bool _Is_debug_enabled_fmt_type(_Basic_format_arg_type _Ty) { + return (_Ty == _Basic_format_arg_type::_Char_type) || (_Ty == _Basic_format_arg_type::_CString_type) + || (_Ty == _Basic_format_arg_type::_String_type); +} +#endif // _HAS_CXX23 + struct _Auto_id_tag {}; // clang-format off @@ -3554,8 +3562,18 @@ struct formatter { _FMT_P2286_BEGIN template struct _Formatter_base { +private: using _Pc = basic_format_parse_context<_CharT>; +public: +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept + requires (_Is_debug_enabled_fmt_type(_ArgType)) + { + _Specs._Type = '?'; + } +#endif // _HAS_CXX23 + constexpr _Pc::iterator parse(_Pc& _ParseCtx) { _Specs_checker<_Dynamic_specs_handler<_Pc>> _Handler(_Dynamic_specs_handler<_Pc>{_Specs, _ParseCtx}, _ArgType); const auto _It = _Parse_format_specs(_ParseCtx._Unchecked_begin(), _ParseCtx._Unchecked_end(), _Handler); diff --git a/tests/std/include/test_format_support.hpp b/tests/std/include/test_format_support.hpp index 77cc36eb857..e0e245d8382 100644 --- a/tests/std/include/test_format_support.hpp +++ b/tests/std/include/test_format_support.hpp @@ -18,6 +18,14 @@ struct choose_literal { static constexpr const char* choose(const char* s, const wchar_t*) { return s; } + + static constexpr char choose(char c, wchar_t) { + return c; + } + + static constexpr std::string_view choose(std::string_view sv, std::wstring_view) { + return sv; + } }; template <> @@ -25,6 +33,14 @@ struct choose_literal { static constexpr const wchar_t* choose(const char*, const wchar_t* s) { return s; } + + static constexpr wchar_t choose(char, wchar_t c) { + return c; + } + + static constexpr std::wstring_view choose(std::string_view, std::wstring_view sv) { + return sv; + } }; #define TYPED_LITERAL(CharT, Literal) (choose_literal::choose(Literal, L##Literal)) diff --git a/tests/std/test.lst b/tests/std/test.lst index 111091a0885..2997f88cf1a 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -581,9 +581,10 @@ tests\P2278R4_const_span tests\P2278R4_ranges_const_iterator_machinery tests\P2278R4_ranges_const_range_machinery tests\P2278R4_views_as_const -tests\P2286R8_formatting_ranges -tests\P2286R8_formatting_ranges_legacy_text_encoding -tests\P2286R8_formatting_ranges_utf8 +tests\P2286R8_text_formatting_debug_enabled_specializations +tests\P2286R8_text_formatting_escaping +tests\P2286R8_text_formatting_escaping_legacy_text_encoding +tests\P2286R8_text_formatting_escaping_utf8 tests\P2302R4_ranges_alg_contains tests\P2302R4_ranges_alg_contains_subrange tests\P2321R2_proxy_reference diff --git a/tests/std/tests/P2286R8_formatting_ranges/env.lst b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/env.lst similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges/env.lst rename to tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/env.lst diff --git a/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp new file mode 100644 index 00000000000..c09e2f69030 --- /dev/null +++ b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp @@ -0,0 +1,205 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define STR(Str) TYPED_LITERAL(CharT, Str) + +using namespace std; + +template +concept DebugEnabledSpecialization = is_default_constructible_v && requires(F& fmt) { + { fmt.set_debug_format() } noexcept -> same_as; +}; + +template +consteval bool check_debug_enabled_specializations() { + static_assert(DebugEnabledSpecialization>); + static_assert(DebugEnabledSpecialization>); + static_assert(DebugEnabledSpecialization>); + static_assert(DebugEnabledSpecialization>); + static_assert(DebugEnabledSpecialization>); + static_assert(DebugEnabledSpecialization, CharT>>); + static_assert(DebugEnabledSpecialization, CharT>>); + static_assert(DebugEnabledSpecialization, CharT>>); + + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + + static_assert(!DebugEnabledSpecialization>); + // NB: formatter is special case, see below + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + static_assert(!DebugEnabledSpecialization>); + + // NB: wchar_t might be defined as a typedef for unsigned short (with '/Zc:wchar_t-') + static_assert(!DebugEnabledSpecialization> + == (same_as || !same_as) ); + + return true; +} + +template +struct Holder { + char narrow_ch; + CharT ch; + const CharT* const_ptr; + basic_string str; + CharT* non_const_ptr; + basic_string_view str_view; + CharT arr[11]; +}; + +// holder-format-specs: +// member debug-format(opt) +// member: +// 0 1 2 ... N (index of member object, single digit) +// debug-format: +// $ (use debug format) +template +struct std::formatter, CharT> { +public: + constexpr formatter() : member_index{-1} {} + + constexpr auto parse(basic_format_parse_context& ctx) { + auto it = ctx.begin(); + if (it == ctx.end() || *it == STR('}')) { + throw format_error{"Invalid holder-format-specs."}; + } + + if (STR('0') <= *it && *it <= STR('9')) { + member_index = *it - STR('0'); + } else { + throw format_error{"Expected member index in holder-format-specs."}; + } + + ++it; + if (it == ctx.end() || *it == STR('}')) { + return it; + } + + if (*it == '$') { + switch (member_index) { + case 0: + fmt0.set_debug_format(); + break; + case 1: + fmt1.set_debug_format(); + break; + case 2: + fmt2.set_debug_format(); + break; + case 3: + fmt3.set_debug_format(); + break; + case 4: + fmt4.set_debug_format(); + break; + case 5: + fmt5.set_debug_format(); + break; + case 6: + fmt6.set_debug_format(); + break; + } + } else { + throw format_error{"Unexpected symbols in holder-format-specs."}; + } + + ++it; + if (it != ctx.end() && *it != STR('}')) { + throw format_error{"Expected '}' at the end of holder-format-specs."}; + } + + return it; + } + + template + auto format(const Holder& val, FormatContext& ctx) const { + switch (member_index) { + case 0: + return fmt0.format(val.narrow_ch, ctx); + case 1: + return fmt1.format(val.ch, ctx); + case 2: + return fmt2.format(val.const_ptr, ctx); + case 3: + return fmt3.format(val.str, ctx); + case 4: + return fmt4.format(val.non_const_ptr, ctx); + case 5: + return fmt5.format(val.str_view, ctx); + case 6: + return fmt6.format(val.arr, ctx); + } + + unreachable(); + } + +private: + int member_index; + + formatter fmt0; + formatter fmt1; + formatter fmt2; + formatter, CharT> fmt3; + formatter fmt4; + formatter, CharT> fmt5; + formatter fmt6; +}; + +template +void check_set_debug_format_function() { + Holder val; + + val.narrow_ch = '\t'; + val.ch = STR('\t'); + val.const_ptr = STR("const\tCharT\t*"); + val.str = STR("basic\tstring"); + val.non_const_ptr = val.str.data(); + val.str_view = STR("basic\tstring\tview"); + ranges::copy(STR("CharT\t[11]\0"sv), val.arr); + + assert(format(STR("{:0}"), val) == STR("\t")); + assert(format(STR("{:1}"), val) == STR("\t")); + assert(format(STR("{:2}"), val) == STR("const\tCharT\t*")); + assert(format(STR("{:3}"), val) == STR("basic\tstring")); + assert(format(STR("{:4}"), val) == STR("basic\tstring")); + assert(format(STR("{:5}"), val) == STR("basic\tstring\tview")); + assert(format(STR("{:6}"), val) == STR("CharT\t[11]")); + + assert(format(STR("{:0$}"), val) == STR(R"('\t')")); + assert(format(STR("{:1$}"), val) == STR(R"('\t')")); + assert(format(STR("{:2$}"), val) == STR(R"("const\tCharT\t*")")); + assert(format(STR("{:3$}"), val) == STR(R"("basic\tstring")")); + assert(format(STR("{:4$}"), val) == STR(R"("basic\tstring")")); + assert(format(STR("{:5$}"), val) == STR(R"("basic\tstring\tview")")); + assert(format(STR("{:6$}"), val) == STR(R"("CharT\t[11]")")); +} + +int main() { + static_assert(check_debug_enabled_specializations()); + static_assert(check_debug_enabled_specializations()); + + check_set_debug_format_function(); + check_set_debug_format_function(); +} diff --git a/tests/std/tests/P2286R8_text_formatting_escaping/env.lst b/tests/std/tests/P2286R8_text_formatting_escaping/env.lst new file mode 100644 index 00000000000..18e2d7c71ec --- /dev/null +++ b/tests/std/tests/P2286R8_text_formatting_escaping/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\concepts_latest_matrix.lst diff --git a/tests/std/tests/P2286R8_formatting_ranges/test.cpp b/tests/std/tests/P2286R8_text_formatting_escaping/test.cpp similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges/test.cpp rename to tests/std/tests/P2286R8_text_formatting_escaping/test.cpp diff --git a/tests/std/tests/P2286R8_formatting_ranges_legacy_text_encoding/env.lst b/tests/std/tests/P2286R8_text_formatting_escaping_legacy_text_encoding/env.lst similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges_legacy_text_encoding/env.lst rename to tests/std/tests/P2286R8_text_formatting_escaping_legacy_text_encoding/env.lst diff --git a/tests/std/tests/P2286R8_formatting_ranges_legacy_text_encoding/test.cpp b/tests/std/tests/P2286R8_text_formatting_escaping_legacy_text_encoding/test.cpp similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges_legacy_text_encoding/test.cpp rename to tests/std/tests/P2286R8_text_formatting_escaping_legacy_text_encoding/test.cpp diff --git a/tests/std/tests/P2286R8_formatting_ranges_utf8/env.lst b/tests/std/tests/P2286R8_text_formatting_escaping_utf8/env.lst similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges_utf8/env.lst rename to tests/std/tests/P2286R8_text_formatting_escaping_utf8/env.lst diff --git a/tests/std/tests/P2286R8_formatting_ranges_utf8/test.cpp b/tests/std/tests/P2286R8_text_formatting_escaping_utf8/test.cpp similarity index 100% rename from tests/std/tests/P2286R8_formatting_ranges_utf8/test.cpp rename to tests/std/tests/P2286R8_text_formatting_escaping_utf8/test.cpp From 332269c894e5ca94d6e79b9b5dd006a03304cd35 Mon Sep 17 00:00:00 2001 From: Jakub Mazurkiewicz Date: Mon, 31 Jul 2023 14:25:26 +0200 Subject: [PATCH 2/3] Fix https://github.com/microsoft/STL/pull/3913#discussion_r1279181961 --- stl/inc/format | 61 ++++++++++++++++--- .../test.cpp | 9 +++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 44f0466d0c8..58316521ae9 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -3567,7 +3567,7 @@ private: public: #if _HAS_CXX23 - constexpr void set_debug_format() noexcept + constexpr void _Set_debug_format() noexcept requires (_Is_debug_enabled_fmt_type(_ArgType)) { _Specs._Type = '?'; @@ -3626,35 +3626,80 @@ _FORMAT_SPECIALIZE_FOR(short, _Basic_format_arg_type::_Int_type); _FORMAT_SPECIALIZE_FOR(unsigned short, _Basic_format_arg_type::_UInt_type); _FORMAT_SPECIALIZE_FOR(long, _Basic_format_arg_type::_Int_type); _FORMAT_SPECIALIZE_FOR(unsigned long, _Basic_format_arg_type::_UInt_type); -_FORMAT_SPECIALIZE_FOR(char, _Basic_format_arg_type::_Char_type); _FORMAT_SPECIALIZE_FOR(signed char, _Basic_format_arg_type::_Int_type); _FORMAT_SPECIALIZE_FOR(unsigned char, _Basic_format_arg_type::_UInt_type); #undef _FORMAT_SPECIALIZE_FOR +// not using the macro because we'd like to add 'set_debug_format' member function in C++23 mode +template <_Format_supported_charT _CharT> +struct formatter : _Formatter_base { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; + // not using the macro because we'd like to avoid the formatter specialization template <> -struct formatter : _Formatter_base {}; +struct formatter : _Formatter_base { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + _Set_debug_format(); + } +#endif // _HAS_CXX23 +}; // We could use the macro for these specializations, but it's confusing to refer to symbols that are defined // inside the macro in the macro's "call". template <_Format_supported_charT _CharT> -struct formatter<_CharT*, _CharT> : _Formatter_base<_CharT*, _CharT, _Basic_format_arg_type::_CString_type> {}; +struct formatter<_CharT*, _CharT> : _Formatter_base<_CharT*, _CharT, _Basic_format_arg_type::_CString_type> { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; template <_Format_supported_charT _CharT> struct formatter - : _Formatter_base {}; + : _Formatter_base { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; template <_Format_supported_charT _CharT, size_t _Nx> -struct formatter<_CharT[_Nx], _CharT> : _Formatter_base<_CharT[_Nx], _CharT, _Basic_format_arg_type::_CString_type> {}; +struct formatter<_CharT[_Nx], _CharT> : _Formatter_base<_CharT[_Nx], _CharT, _Basic_format_arg_type::_CString_type> { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; template <_Format_supported_charT _CharT, class _Traits, class _Allocator> struct formatter, _CharT> - : _Formatter_base, _CharT, _Basic_format_arg_type::_String_type> {}; + : _Formatter_base, _CharT, _Basic_format_arg_type::_String_type> { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; template <_Format_supported_charT _CharT, class _Traits> struct formatter, _CharT> - : _Formatter_base, _CharT, _Basic_format_arg_type::_String_type> {}; + : _Formatter_base, _CharT, _Basic_format_arg_type::_String_type> { +#if _HAS_CXX23 + constexpr void set_debug_format() noexcept { + this->_Set_debug_format(); + } +#endif // _HAS_CXX23 +}; _EXPORT_STD template struct basic_format_string { diff --git a/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp index c09e2f69030..66f53f27292 100644 --- a/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp @@ -196,6 +196,15 @@ void check_set_debug_format_function() { assert(format(STR("{:6$}"), val) == STR(R"("CharT\t[11]")")); } +void set_debug_format(auto&) {} + +struct name_lookup_in_formatter_checker : formatter { + auto parse(auto& ctx) { // COMPILE-ONLY + set_debug_format(*this); + return ctx.begin(); + } +}; + int main() { static_assert(check_debug_enabled_specializations()); static_assert(check_debug_enabled_specializations()); From 6dc2a4611d57f312b10ebe6d94c792b1b2961a88 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 2 Aug 2023 17:34:20 -0700 Subject: [PATCH 3/3] Code review feedback. --- stl/inc/format | 4 ++-- .../test.cpp | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index 58316521ae9..5ba65560b31 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -132,8 +132,8 @@ _NODISCARD constexpr bool _Is_arithmetic_fmt_type(_Basic_format_arg_type _Ty) { #if _HAS_CXX23 _NODISCARD consteval bool _Is_debug_enabled_fmt_type(_Basic_format_arg_type _Ty) { - return (_Ty == _Basic_format_arg_type::_Char_type) || (_Ty == _Basic_format_arg_type::_CString_type) - || (_Ty == _Basic_format_arg_type::_String_type); + return _Ty == _Basic_format_arg_type::_Char_type || _Ty == _Basic_format_arg_type::_CString_type + || _Ty == _Basic_format_arg_type::_String_type; } #endif // _HAS_CXX23 diff --git a/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp index 66f53f27292..d5ac1fad875 100644 --- a/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp +++ b/tests/std/tests/P2286R8_text_formatting_debug_enabled_specializations/test.cpp @@ -52,8 +52,7 @@ consteval bool check_debug_enabled_specializations() { static_assert(!DebugEnabledSpecialization>); // NB: wchar_t might be defined as a typedef for unsigned short (with '/Zc:wchar_t-') - static_assert(!DebugEnabledSpecialization> - == (same_as || !same_as) ); + static_assert(DebugEnabledSpecialization> == same_as); return true; } @@ -78,8 +77,6 @@ struct Holder { template struct std::formatter, CharT> { public: - constexpr formatter() : member_index{-1} {} - constexpr auto parse(basic_format_parse_context& ctx) { auto it = ctx.begin(); if (it == ctx.end() || *it == STR('}')) { @@ -156,7 +153,7 @@ struct std::formatter, CharT> { } private: - int member_index; + int member_index{-1}; formatter fmt0; formatter fmt1;