From f5c14f8a5ac7be3b73a2dba448ed3fc2468e3f02 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 16 Jan 2023 09:42:43 +0800 Subject: [PATCH 1/7] Test coverage for LWG-3857 --- tests/std/tests/P0220R1_string_view/test.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0220R1_string_view/test.cpp b/tests/std/tests/P0220R1_string_view/test.cpp index 0e550f1b0fc..78324c1c576 100644 --- a/tests/std/tests/P0220R1_string_view/test.cpp +++ b/tests/std/tests/P0220R1_string_view/test.cpp @@ -367,8 +367,18 @@ constexpr bool test_case_range_constructor() { static_assert(!is_constructible_v>); // different elements static_assert(!is_convertible_v, string_view>); - static_assert(!is_constructible_v>); // different traits - static_assert(!is_convertible_v, string_view>); + static_assert(!is_convertible_v, string_view>); // different traits + static_assert(!is_convertible_v, string_view>); + + static_assert(!is_convertible_v>); + static_assert(!is_convertible_v, string>); + + // LWG-3857 basic_string_view should allow explicit conversion when only traits vary + static_assert(is_constructible_v>); + static_assert(is_constructible_v, string_view>); + + static_assert(is_constructible_v>); + static_assert(is_constructible_v, string>); #endif // _HAS_CXX23 && defined(__cpp_lib_concepts) return true; From 12d513d3b2d54fb16fd02158b78944672192ff5a Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 16 Jan 2023 09:44:53 +0800 Subject: [PATCH 2/7] Speculatively implement LWG-3857 --- stl/inc/xstring | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/stl/inc/xstring b/stl/inc/xstring index 23476903226..a51a3df7de4 100644 --- a/stl/inc/xstring +++ b/stl/inc/xstring @@ -1260,10 +1260,7 @@ public: && (!is_convertible_v<_Range, const _Elem*>) && (!requires(remove_cvref_t<_Range>& _Rng) { _Rng.operator _STD basic_string_view<_Elem, _Traits>(); - }) - && (!requires { - typename remove_reference_t<_Range>::traits_type; - } || same_as::traits_type, _Traits>)) + })) // doesn't check member type traits_type, per LWG-3857 constexpr explicit basic_string_view(_Range&& _Rng) noexcept( noexcept(_RANGES data(_Rng)) && noexcept(_RANGES size(_Rng))) // strengthened : _Mydata(_RANGES data(_Rng)), _Mysize(static_cast(_RANGES size(_Rng))) {} From 3b4efc8533e6d083c92b4b1aab0970566adf56d5 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 16 Jan 2023 10:09:21 +0800 Subject: [PATCH 3/7] Skip several libcxx tests due to LWG-3857 --- tests/libcxx/expected_results.txt | 5 +++++ tests/libcxx/skipped_tests.txt | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index d1d5b5ed10f..34db4834575 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -607,6 +607,11 @@ std/language.support/support.limits/support.limits.general/format.version.compil # libc++ doesn't yet implement LWG-3670 std/ranges/range.factories/range.iota.view/iterator/member_typedefs.compile.pass.cpp FAIL +# libc++ doesn't speculatively implement LWG-3857 +std/strings/string.view/string.view.cons/from_range.pass.cpp FAIL +std/strings/string.view/string.view.cons/from_string1.compile.fail.cpp FAIL +std/strings/string.view/string.view.cons/from_string2.compile.fail.cpp FAIL + # This test assumes that std::array::iterator is int*, but on MSVC STL it's _Array_iterator. # Other std/algorithm test failures likely have the same cause. std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.pass.cpp FAIL diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index ea976d99489..a75a5571413 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -607,6 +607,11 @@ language.support\support.limits\support.limits.general\format.version.compile.pa # libc++ doesn't yet implement LWG-3670 ranges\range.factories\range.iota.view\iterator\member_typedefs.compile.pass.cpp +# libc++ doesn't speculatively implement LWG-3857 +strings\string.view\string.view.cons\from_range.pass.cpp +strings\string.view\string.view.cons\from_string1.compile.fail.cpp +strings\string.view\string.view.cons\from_string2.compile.fail.cpp + # This test assumes that std::array::iterator is int*, but on MSVC STL it's _Array_iterator. # Other std/algorithm test failures likely have the same cause. algorithms\alg.modifying.operations\alg.copy\ranges.copy.pass.cpp From ca2d4f31ebdfc2f9a88e762cd7f2df1c7140fed9 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 18 Jan 2023 16:28:53 +0800 Subject: [PATCH 4/7] Alternative fixing method is needed in C++20 --- stl/inc/format | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/stl/inc/format b/stl/inc/format index ef95bbfc059..b758c0dadfb 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -1783,6 +1783,20 @@ struct _Format_arg_traits { template static constexpr size_t _Storage_size = sizeof(_Storage_type<_Ty>); + +#if !_HAS_CXX23 + // Workaround towards N4928 [format.arg]/9 and /10 in C++20 + template + static auto _Deduce_converted_string_type(basic_string_view<_Char_type, _Traits>) + -> basic_string_view<_Char_type, _Traits>; // not defined + + template + static auto _Deduce_converted_string_type(basic_string<_Char_type, _Traits, _Alloc>) + -> const basic_string<_Char_type, _Traits, _Alloc>&; // not defined + + template + using _Converted_string_type = decltype(_Deduce_converted_string_type(_STD declval<_Ty&>())); +#endif // !_HAS_CXX23 }; struct _Format_arg_index { @@ -1878,7 +1892,16 @@ private: _Arg_type = _Basic_format_arg_type::_Custom_type; } - _Store_impl<_Erased_type>(_Arg_index, _Arg_type, static_cast<_Erased_type>(_Val)); +#if !_HAS_CXX23 + // Workaround towards N4928 [format.arg]/9 and /10 in C++20 + if constexpr (is_same_v<_Erased_type, basic_string_view<_CharType>>) { + typename _Traits::template _Converted_string_type<_Ty> _Str = _Val; + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type{_Str.data(), _Str.size()}); + } else +#endif // !_HAS_CXX23 + { + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, static_cast<_Erased_type>(_Val)); + } } public: From 760e925287472362ecbb23574fb6010574a99af6 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 18 Jan 2023 17:20:02 +0800 Subject: [PATCH 5/7] Address @cpplearner's review comments And update references to WG21-N4928. Co-authored-by: S. B. Tam --- stl/inc/format | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/stl/inc/format b/stl/inc/format index b758c0dadfb..1e7feb962c5 100644 --- a/stl/inc/format +++ b/stl/inc/format @@ -853,13 +853,13 @@ _NODISCARD constexpr bool _Is_execution_charset_self_synchronizing() { #endif // ^^^ EDG workaround ^^^ } -inline constexpr char32_t _Width_estimate_intervals[] = { // Per N4885 [format.string.std]/11 +inline constexpr char32_t _Width_estimate_intervals[] = { // Per N4928 [format.string.std]/12 0x1100u, 0x1160u, 0x2329u, 0x232Bu, 0x2E80u, 0x303Fu, 0x3040u, 0xA4D0u, 0xAC00u, 0xD7A4u, 0xF900u, 0xFB00u, 0xFE10u, 0xFE1Au, 0xFE30u, 0xFE70u, 0xFF00u, 0xFF61u, 0xFFE0u, 0xFFE7u, 0x1F300u, 0x1F650u, 0x1F900u, 0x1FA00u, 0x20000u, 0x2FFFEu, 0x30000u, 0x3FFFEu}; _NODISCARD constexpr int _Unicode_width_estimate(const char32_t _Ch) noexcept { - // Computes the width estimation for Unicode characters from N4885 [format.string.std]/11 + // Computes the width estimation for Unicode characters from N4928 [format.string.std]/12 int _Result = 1; for (const auto& _Bound : _Width_estimate_intervals) { if (_Ch < _Bound) { @@ -1731,13 +1731,13 @@ struct _Format_arg_traits { using _Char_type = typename _Context::char_type; // These overloads mirror the exposition-only single-argument constructor - // set of basic_format_arg (N4885 [format.arg]). They determine the mapping + // set of basic_format_arg (N4928 [format.arg]). They determine the mapping // from "raw" to "erased" argument type for _Format_arg_store. template <_Has_formatter<_Context> _Ty> static auto _Phony_basic_format_arg_constructor(_Ty&&) { // per the proposed resolution of LWG-3631 using _Td = remove_cvref_t<_Ty>; - // See N4885 [format.arg]/5 + // See N4928 [format.arg]/5 if constexpr (is_same_v<_Td, bool>) { return bool{}; } else if constexpr (is_same_v<_Td, _Char_type>) { @@ -1783,20 +1783,6 @@ struct _Format_arg_traits { template static constexpr size_t _Storage_size = sizeof(_Storage_type<_Ty>); - -#if !_HAS_CXX23 - // Workaround towards N4928 [format.arg]/9 and /10 in C++20 - template - static auto _Deduce_converted_string_type(basic_string_view<_Char_type, _Traits>) - -> basic_string_view<_Char_type, _Traits>; // not defined - - template - static auto _Deduce_converted_string_type(basic_string<_Char_type, _Traits, _Alloc>) - -> const basic_string<_Char_type, _Traits, _Alloc>&; // not defined - - template - using _Converted_string_type = decltype(_Deduce_converted_string_type(_STD declval<_Ty&>())); -#endif // !_HAS_CXX23 }; struct _Format_arg_index { @@ -1895,8 +1881,7 @@ private: #if !_HAS_CXX23 // Workaround towards N4928 [format.arg]/9 and /10 in C++20 if constexpr (is_same_v<_Erased_type, basic_string_view<_CharType>>) { - typename _Traits::template _Converted_string_type<_Ty> _Str = _Val; - _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type{_Str.data(), _Str.size()}); + _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type{_Val.data(), _Val.size()}); } else #endif // !_HAS_CXX23 { @@ -3291,7 +3276,7 @@ struct _Format_handler { }; // Generic formatter definition, the deleted default constructor -// makes it "disabled" as per N4885 [format.formatter.spec]/5 +// makes it "disabled" as per N4928 [format.formatter.spec]/5 _EXPORT_STD template struct formatter { formatter() = delete; @@ -3420,7 +3405,7 @@ _EXPORT_STD template _NODISCARD auto make_format_args(_Args&&... _Vals) { static_assert((_Has_formatter<_Args, _Context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4917 [format.arg.store]/2 and [formatter.requirements]."); + "See N4928 [format.arg.store]/2 and [formatter.requirements]."); return _Format_arg_store<_Context, _Args...>{_Vals...}; } @@ -3428,7 +3413,7 @@ _EXPORT_STD template _NODISCARD auto make_wformat_args(_Args&&... _Vals) { static_assert((_Has_formatter<_Args, wformat_context> && ...), "Cannot format an argument. To make type T formattable, provide a formatter specialization. " - "See N4917 [format.arg.store]/2 and [formatter.requirements]."); + "See N4928 [format.arg.store]/2 and [formatter.requirements]."); return _Format_arg_store{_Vals...}; } From a86e821af5925fc3e54252c6619dce09834737f3 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 18 Jan 2023 17:57:46 +0800 Subject: [PATCH 6/7] Test coverage for non-Standard traits_type --- .../test.cpp | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index 3cfae561363..bc6e63ce9e5 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -25,7 +25,7 @@ constexpr auto llong_max = numeric_limits::max(); constexpr auto ullong_max = numeric_limits::max(); // copied from the string_view tests -template +template struct choose_literal; // not defined template <> @@ -52,6 +52,16 @@ struct choose_literal { #define DEFAULT_IDL_SETTING 0 #endif +// Test formatting basic_string(_view) with non-Standard traits_type +template +struct alternative_char_traits : char_traits {}; + +template +using alternative_basic_string_view = basic_string_view>; + +template > +using alternative_basic_string = basic_string, Alloc>; + template auto make_testing_format_args(Args&&... vals) { if constexpr (is_same_v) { @@ -136,6 +146,26 @@ void test_simple_formatting() { 0.0, STR("s"), basic_string_view{STR("sv")}, nullptr, static_cast(nullptr)); assert(output_string == STR("true a 0 0 0 s sv 0x0 0x0")); + // Test formatting basic_string(_view) with non-Standard traits_type + // TRANSITION, LLVM-54051, DevCom-10255929, should also test class template argument deduction for alias templates + output_string.clear(); + format_to(move_only_back_inserter{output_string}, STR("{} {} {} {} {} {} {} {} {} {}"), true, charT{'a'}, 0, 0u, + 0.0, STR("s"), alternative_basic_string{STR("str")}, alternative_basic_string_view{STR("sv")}, + nullptr, static_cast(nullptr)); + assert(output_string == STR("true a 0 0 0 s str sv 0x0 0x0")); + + output_string.clear(); + format_to(move_only_back_inserter{output_string}, STR("{:} {:} {:} {:} {:} {:} {:} {:} {:} {:}"), true, charT{'a'}, + 0, 0u, 0.0, STR("s"), alternative_basic_string{STR("str")}, + alternative_basic_string_view{STR("sv")}, nullptr, static_cast(nullptr)); + assert(output_string == STR("true a 0 0 0 s str sv 0x0 0x0")); + + output_string.clear(); + format_to_n(move_only_back_inserter{output_string}, 300, STR("{} {} {} {} {} {} {} {} {} {}"), true, charT{'a'}, 0, + 0u, 0.0, STR("s"), alternative_basic_string{STR("str")}, alternative_basic_string_view{STR("sv")}, + nullptr, static_cast(nullptr)); + assert(output_string == STR("true a 0 0 0 s str sv 0x0 0x0")); + output_string.clear(); vformat_to( back_insert_iterator{output_string}, locale::classic(), STR("format"), make_testing_format_args()); From b50437f4a9f08add03f6a8782f466f087306dacf Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 19 Jan 2023 14:57:48 -0800 Subject: [PATCH 7/7] Remove unnecessary std qualification. --- tests/std/tests/P0645R10_text_formatting_formatting/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp index bc6e63ce9e5..fabf6cde482 100644 --- a/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp +++ b/tests/std/tests/P0645R10_text_formatting_formatting/test.cpp @@ -59,7 +59,7 @@ struct alternative_char_traits : char_traits {}; template using alternative_basic_string_view = basic_string_view>; -template > +template > using alternative_basic_string = basic_string, Alloc>; template