[libcxx-commits] [libcxx] 04a3146 - [libc++][format] Fixes string-literal formatting.

[Mark de Wever via libcxx-commits]

Author: Mark de Wever
Date: 2022-06-01T17:49:09+02:00
New Revision: 04a3146caa0f25014c3c9c5ebce5b5a292d77fc9

URL: https://github.com/llvm/llvm-project/commit/04a3146caa0f25014c3c9c5ebce5b5a292d77fc9
DIFF: https://github.com/llvm/llvm-project/commit/04a3146caa0f25014c3c9c5ebce5b5a292d77fc9.diff

LOG: [libc++][format] Fixes string-literal formatting.

Formatting a string-literal had an off-by-one issue where the NUL
terminator became part of the formatted output.

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D126665

Added: 
    

Modified: 
    libcxx/include/__format/format_arg_store.h
    libcxx/test/std/utilities/format/format.functions/format_tests.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__format/format_arg_store.h b/libcxx/include/__format/format_arg_store.h
index 4a3fcd4ff339d..28c9c28d96671 100644
--- a/libcxx/include/__format/format_arg_store.h
+++ b/libcxx/include/__format/format_arg_store.h
@@ -17,8 +17,6 @@
 #include <__config>
 #include <__format/concepts.h>
 #include <__format/format_arg.h>
-#include <__iterator/data.h>
-#include <__iterator/size.h>
 #include <cstring>
 #include <string>
 #include <string_view>
@@ -173,13 +171,15 @@ _LIBCPP_HIDE_FROM_ABI basic_format_arg<_Context> __create_format_arg(_Tp&& __val
   else if constexpr (__arg == __arg_t::__unsigned_long_long)
     return basic_format_arg<_Context>{__arg, static_cast<unsigned long long>(__value)};
   else if constexpr (__arg == __arg_t::__string_view)
-    // When the _Traits or _Allocator are 
diff erent an implicit conversion will
-    // fail.
-    //
-    // Note since the input can be an array use the non-member functions to
-    // extract the constructor arguments.
-    return basic_format_arg<_Context>{
-        __arg, basic_string_view<typename _Context::char_type>{_VSTD::data(__value), _VSTD::size(__value)}};
+    // Using std::size on a character array will add the NUL-terminator to the size.
+    if constexpr (is_array_v<remove_cvref_t<_Tp>>)
+      return basic_format_arg<_Context>{
+          __arg, basic_string_view<typename _Context::char_type>{__value, extent_v<remove_cvref_t<_Tp>> - 1}};
+    else
+      // When the _Traits or _Allocator are 
diff erent an implicit conversion will
+      // fail.
+      return basic_format_arg<_Context>{
+          __arg, basic_string_view<typename _Context::char_type>{__value.data(), __value.size()}};
   else if constexpr (__arg == __arg_t::__ptr)
     return basic_format_arg<_Context>{__arg, static_cast<const void*>(__value)};
   else if constexpr (__arg == __arg_t::__handle)

diff  --git a/libcxx/test/std/utilities/format/format.functions/format_tests.h b/libcxx/test/std/utilities/format/format.functions/format_tests.h
index 0c6993595b5e9..4ee580bb2a942 100644
--- a/libcxx/test/std/utilities/format/format.functions/format_tests.h
+++ b/libcxx/test/std/utilities/format/format.functions/format_tests.h
@@ -174,8 +174,10 @@ case #T[0]:
   return result;
 }
 
-template <class CharT, class T, class TestFunction, class ExceptionTest>
-void format_test_string(T world, T universe, TestFunction check, ExceptionTest check_exception) {
+// Using a const ref for world and universe so a string literal will be a character array.
+// When passed as character array W and U have 
diff erent types.
+template <class CharT, class W, class U, class TestFunction, class ExceptionTest>
+void format_test_string(const W& world, const U& universe, TestFunction check, ExceptionTest check_exception) {
 
   // *** Valid input tests ***
   // Unsed argument is ignored. TODO FMT what does the Standard mandate?
@@ -291,6 +293,44 @@ template <class CharT, class TestFunction>
 void format_test_string_unicode(TestFunction check) {
   (void)check;
 #ifndef TEST_HAS_NO_UNICODE
+  // Make sure all possible types are tested. For clarity don't use macros.
+  if constexpr (std::same_as<CharT, char>) {
+    const char* c_string = "aßc";
+    check.template operator()<"{:*^5}">(SV("*aßc*"), c_string);
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), c_string);
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), const_cast<char*>(c_string));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), const_cast<char*>(c_string));
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), "aßc");
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), "aßc");
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), std::string("aßc"));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), std::string("aßc"));
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), std::string_view("aßc"));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), std::string_view("aßc"));
+  }
+#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  else {
+    const wchar_t* c_string = L"aßc";
+    check.template operator()<"{:*^5}">(SV("*aßc*"), c_string);
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), c_string);
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), const_cast<wchar_t*>(c_string));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), const_cast<wchar_t*>(c_string));
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), L"aßc");
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), L"aßc");
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), std::wstring(L"aßc"));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), std::wstring(L"aßc"));
+
+    check.template operator()<"{:*^5}">(SV("*aßc*"), std::wstring_view(L"aßc"));
+    check.template operator()<"{:*^4.2}">(SV("*aß*"), std::wstring_view(L"aßc"));
+  }
+#  endif
+
   // ß requires one column
   check.template operator()<"{}">(SV("aßc"), STR("aßc"));
 
@@ -330,9 +370,14 @@ void format_string_tests(TestFunction check, ExceptionTest check_exception) {
   std::basic_string<CharT> world = STR("world");
   std::basic_string<CharT> universe = STR("universe");
 
-  // Testing the char const[] is a bit tricky due to array to pointer decay.
-  // Since there are separate tests in format.formatter.spec the array is not
-  // tested here.
+  // Test a string literal in a way it won't decay to a pointer.
+  if constexpr (std::same_as<CharT, char>)
+    format_test_string<CharT>("world", "universe", check, check_exception);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  else
+    format_test_string<CharT>(L"world", L"universe", check, check_exception);
+#endif
+
   format_test_string<CharT>(world.c_str(), universe.c_str(), check, check_exception);
   format_test_string<CharT>(const_cast<CharT*>(world.c_str()), const_cast<CharT*>(universe.c_str()), check,
                             check_exception);