Skip to content

Commit

Permalink
[libc++][format] Fixes string-literal formatting.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mordante committed Jun 1, 2022
1 parent fe2cc16 commit 04a3146
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 14 deletions.
18 changes: 9 additions & 9 deletions libcxx/include/__format/format_arg_store.h
Expand Up @@ -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>
Expand Down Expand Up @@ -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 different 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 different 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)
Expand Down
55 changes: 50 additions & 5 deletions libcxx/test/std/utilities/format/format.functions/format_tests.h
Expand Up @@ -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 different 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?
Expand Down Expand Up @@ -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"));

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 04a3146

Please sign in to comment.