Speculatively implement LWG-3857 (basic_string_view conversions) (#3348)

Co-authored-by: S. B. Tam <[email protected]>
Co-authored-by: Stephan T. Lavavej <[email protected]>

Author: 3 people

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>) {
@@ -1878,7 +1878,15 @@ 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>>) {
+            _Store_impl<_Erased_type>(_Arg_index, _Arg_type, _Erased_type{_Val.data(), _Val.size()});
+        } else
+#endif // !_HAS_CXX23
+        {
+            _Store_impl<_Erased_type>(_Arg_index, _Arg_type, static_cast<_Erased_type>(_Val));
+        }
     }
 
 public:
@@ -3268,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 <class _Ty, class _CharT>
 struct formatter {
     formatter()                           = delete;
@@ -3397,15 +3405,15 @@ _EXPORT_STD template <class _Context = format_context, class... _Args>
 _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<T> 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...};
 }
 
 _EXPORT_STD template <class... _Args>
 _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<T> specialization. "
-        "See N4917 [format.arg.store]/2 and [formatter.requirements].");
+        "See N4928 [format.arg.store]/2 and [formatter.requirements].");
     return _Format_arg_store<wformat_context, _Args...>{_Vals...};
 }
 

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<typename remove_reference_t<_Range>::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<size_t>(_RANGES size(_Rng))) {}

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<int, 4>::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

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<int, 4>::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

tests/std/tests/P0220R1_string_view/test.cpp

@@ -367,8 +367,18 @@ constexpr bool test_case_range_constructor() {
     static_assert(!is_constructible_v<string_view, vector<unsigned char>>); // different elements
     static_assert(!is_convertible_v<vector<unsigned char>, string_view>);
 
-    static_assert(!is_constructible_v<string_view, basic_string<char, constexpr_char_traits>>); // different traits
-    static_assert(!is_convertible_v<basic_string<char, constexpr_char_traits>, string_view>);
+    static_assert(!is_convertible_v<basic_string<char, constexpr_char_traits>, string_view>); // different traits
+    static_assert(!is_convertible_v<basic_string_view<char, constexpr_char_traits>, string_view>);
+
+    static_assert(!is_convertible_v<string_view, basic_string_view<char, constexpr_char_traits>>);
+    static_assert(!is_convertible_v<basic_string_view<char, constexpr_char_traits>, string>);
+
+    // LWG-3857 basic_string_view should allow explicit conversion when only traits vary
+    static_assert(is_constructible_v<string_view, basic_string<char, constexpr_char_traits>>);
+    static_assert(is_constructible_v<basic_string_view<char, constexpr_char_traits>, string_view>);
+
+    static_assert(is_constructible_v<string_view, basic_string_view<char, constexpr_char_traits>>);
+    static_assert(is_constructible_v<basic_string_view<char, constexpr_char_traits>, string>);
 #endif // _HAS_CXX23 && defined(__cpp_lib_concepts)
 
     return true;

tests/std/tests/P0645R10_text_formatting_formatting/test.cpp

@@ -25,7 +25,7 @@ constexpr auto llong_max  = numeric_limits<long long>::max();
 constexpr auto ullong_max = numeric_limits<unsigned long long>::max();
 
 // copied from the string_view tests
-template <typename CharT>
+template <class CharT>
 struct choose_literal; // not defined
 
 template <>
@@ -52,6 +52,16 @@ struct choose_literal<wchar_t> {
 #define DEFAULT_IDL_SETTING 0
 #endif
 
+// Test formatting basic_string(_view) with non-Standard traits_type
+template <class CharT>
+struct alternative_char_traits : char_traits<CharT> {};
+
+template <class CharT>
+using alternative_basic_string_view = basic_string_view<CharT, alternative_char_traits<CharT>>;
+
+template <class CharT, class Alloc = allocator<CharT>>
+using alternative_basic_string = basic_string<CharT, alternative_char_traits<CharT>, Alloc>;
+
 template <class charT, class... Args>
 auto make_testing_format_args(Args&&... vals) {
     if constexpr (is_same_v<charT, wchar_t>) {
@@ -136,6 +146,26 @@ void test_simple_formatting() {
         0.0, STR("s"), basic_string_view{STR("sv")}, nullptr, static_cast<void*>(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<charT>{STR("str")}, alternative_basic_string_view<charT>{STR("sv")},
+        nullptr, static_cast<void*>(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<charT>{STR("str")},
+        alternative_basic_string_view<charT>{STR("sv")}, nullptr, static_cast<void*>(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<charT>{STR("str")}, alternative_basic_string_view<charT>{STR("sv")},
+        nullptr, static_cast<void*>(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<charT>());