[libc++][format] Adds formatter<charT[N], charT>.

[Mordante]

Summary

This formatter isn't in the list of required formatters in

[format.formatter.spec]/2.2

For each charT, the string type specializations
 template<> struct formatter<charT*, charT>;
template<> struct formatter<const charT*, charT>;
template<size_t N> struct formatter<const charT[N], charT>;
template<class traits, class Allocator>
  struct formatter<basic_string<charT, traits, Allocator>, charT>;
template<class traits>
  struct formatter<basic_string_view<charT, traits>, charT>;

Since remove_cvref_t<const charT[N]> is charT[N] the formatter is
required by

[format.functions]/25

Preconditions: formatter<remove_cvref_t<Ti>, charT> meets the
BasicFormatter requirements ([formatter.requirements]) for each Ti in
Args.

Depends on D120921

[• Quuxplusone]

libcxx/include/__format/formatter_string.h

120

This formatter isn't in the list of required formatters in https://eel.is/c++draft/format.formatter.spec#2.2 [...]

For each charT, the string type specializations
template<> struct formatter<charT*, charT>;
template<> struct formatter<const charT*, charT>;
template<size_t N> struct formatter<const charT[N], charT>;

This strikes me as a minor defect in the standard wording; @vitaut what do you think? Should the list of required formatters in https://eel.is/c++draft/format.formatter.spec#2.2 simply be updated to mention

template<size_t N> struct formatter<charT[N], charT>;

and then leave

template<size_t N> struct formatter<const charT[N], charT>;

to be covered automatically by... um... wait... does formatter's specification not give any partial specializations to deal with cvref-qualified types? Is that a problem? Or is it actually OK if formatter<const char[N]>, formatter<const int&>, etc, fail to exist, because the STL code will never try to use them anyway? It looks like the STL code only ever tries to use formatter<remove_cvref_t<T>>. I don't think the exact formatting algorithm is specified anywhere in the standard, although https://eel.is/c++draft/format#functions-20 gives a good hint as to the intended implementation.

Anyway, I would:

• treat this as a bug in the standard
• file an LWG issue about it
• just eliminate the bogus const on old line 113/new line 124, instead of adding a whole new partial specialization

128

Pre-existing, but please fix this at the same time: https://quuxplusone.github.io/blog/2020/03/12/contra-array-parameters/
This should be

_LIBCPP_HIDE_FROM_ABI auto format(const _CharT *__str, auto& __ctx)

(Note, even when the const on line 113/124 is removed, the const on line 117/128 will remain, because we don't intend to modify the pointee.)

[Mordante]

Retry CI since passes locally.

[Mordante]

libcxx/include/__format/formatter_string.h

120

The formatter is indeed not used, but without it a string literal isn't formattable. Which is an issue for the next patch in this series.

I had a private conversation with Victor and Charlie about this a while back. Removing const char[N] is an ABI break and an issue for MSVC STL. So removing it will cause implementation divergence. Victor thought it wasn't needed to file an LWG issue. (I'm somewhat on the fence about filing it.)

128

In this case I prefer to keep it as is. There's a separate formatter for const char* and char*. So here using the array shows it's a different function. Also the size is correct, when it's wrong there's UB since it's used in the constructor of the basic_string_view.

[• Quuxplusone]

libcxx/include/__format/formatter_string.h

128

Also the size is correct, when it's wrong there's UB

No, that's not true. See https://quuxplusone.github.io/blog/2020/03/12/contra-array-parameters/
In both C and C++, a function type of the form int(int[42]) is exactly, 100%, equivalent to int(int*). As Linus says, writing the pointer parameter using square-bracket syntax is misleading and can lead only to further mistakes.

[• Quuxplusone]

libcxx/include/__format/formatter_string.h

120

@vitaut @barcharcraz , I think we should file an LWG issue about this. There's no point in the standard specifying a type that literally can't be used, while meanwhile failing to specify a type that is actually necessary for the entire machine to work correctly. Institutional knowledge is great, but if the standard means formatter<char[N]>, it should actually say that; it shouldn't leave it up to a consensus of the vendors to understand that "well, that one const is just kidding."

This has also made me much more confident that we should remove the specialization of formatter<const char[N]> at the same time, so that people don't start relying on it. (This is the same line of argument that leads me to want us to const-qualify our format functions even though that LWG issue is still open.)

[vitaut]

libcxx/include/__format/formatter_string.h

120

I think we should file an LWG issue about this.

Please do. I agree with your analysis that const shouldn't be there which follows from https://eel.is/c++draft/format#functions-20.

[Mordante]

Rebased and removed libcpp-no-concepts from tests.

[Mordante]

libcxx/include/__format/formatter_string.h

120

I'll file an LWG issue.

[vitaut]

libcxx/include/__format/formatter_string.h

111–113

I suggest adding a static assert that _CharT is non-const.

[Mordante]

Address review comments.

[Mordante]

Rebased for next patches in series.

[Mordante]

Rebase to trigger CI

[Mordante]

libcxx/include/__format/formatter_string.h

120

I've filed an LWG issue yesterday.

libcxx/include/__format/formatter_string.h

struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT
   using _Base = formatter<const _CharT*, _CharT>;
 
   _LIBCPP_HIDE_FROM_ABI auto format(_CharT* __str, auto& __ctx)
       -> decltype(__ctx.out()) {
     return _Base::format(__str, __ctx);
   }
 };
 
+// Formatter char[].
+template <__formatter::__char_type _CharT, size_t _Size>
+struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT formatter<_CharT[_Size], _CharT>
+    : public __format_spec::__formatter_string<_CharT> {
+  using _Base = __format_spec::__formatter_string<_CharT>;
+
+  _LIBCPP_HIDE_FROM_ABI auto format(_CharT __str[_Size], auto& __ctx) -> decltype(__ctx.out()) {
+    return _Base::format(basic_string_view<_CharT>(__str, _Size), __ctx);
+  }
+};
+
 // Formatter const char[].
 template <__formatter::__char_type _CharT, size_t _Size>
 struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT
     formatter<const _CharT[_Size], _CharT>
     : public __format_spec::__formatter_string<_CharT> {
   using _Base = __format_spec::__formatter_string<_CharT>;
 
   _LIBCPP_HIDE_FROM_ABI auto format(const _CharT __str[_Size], auto& __ctx)
       -> decltype(__ctx.out()) {
     return _Base::format(basic_string_view<_CharT>(__str, _Size), __ctx);
   }
 };
 
 // Formatter std::string.
 template <__formatter::__char_type _CharT, class _Traits, class _Allocator>
 struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT

libcxx/test/libcxx/utilities/format/format.formatter/format.formatter.spec/formattable.compile.pass.cpp

 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   // Tests the special formatter that converts a char to a wchar_t.
   assert_is_formattable<char, wchar_t>();
 #endif
   assert_is_formattable<CharT, CharT>();
 
   assert_is_formattable<CharT*, CharT>();
   assert_is_formattable<const CharT*, CharT>();
+  assert_is_formattable<CharT[42], CharT>();
   assert_is_formattable<std::basic_string<CharT>, CharT>();
   assert_is_formattable<std::basic_string_view<CharT>, CharT>();
 
   assert_is_formattable<bool, CharT>();
 
   assert_is_formattable<signed char, CharT>();
   assert_is_formattable<signed short, CharT>();
   assert_is_formattable<signed int, CharT>();

libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char_array.pass.cpp

+//===----------------------------------------------------------------------===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-format
+// TODO FMT Evaluate gcc-11 status
+// UNSUPPORTED: gcc-11
+// UNSUPPORTED: apple-clang-12
+
+// <format>
+
+// [format.functions]/25
+//   Preconditions: formatter<remove_cvref_t<Ti>, charT> meets the
+//   BasicFormatter requirements ([formatter.requirements]) for each Ti in
+//   Args.
+
+#include <format>
+#include <cassert>
+#include <concepts>
+#include <type_traits>
+
+#include "test_macros.h"
+#include "make_string.h"
+
+#define STR(S) MAKE_STRING(CharT, S)
+#define CSTR(S) MAKE_CSTRING(CharT, S)
+
+// This is based on the method found in
+// clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-cxx20.cpp
+template <size_t N>
+struct Tester {
+  // This is not part of the real test, but is used the deduce the size of the input.
+  constexpr Tester(const char (&r)[N]) { __builtin_memcpy(text, r, N); }
+  char text[N];
+
+  // The size of the array shouldn't include the NUL character.
+  static const size_t size = N - 1;
+
+  template <class CharT>
+  void test(const std::basic_string<CharT>& expected, const std::basic_string_view<CharT>& fmt) const {
+    using Str = CharT[size];
+    std::basic_format_parse_context<CharT> parse_ctx{fmt};
+    std::formatter<Str, CharT> formatter;
+    static_assert(std::semiregular<decltype(formatter)>);
+
+    auto it = formatter.parse(parse_ctx);
+    assert(it == fmt.end() - (!fmt.empty() && fmt.back() == '}'));
+
+    std::basic_string<CharT> result;
+    auto out = std::back_inserter(result);
+    using FormatCtxT = std::basic_format_context<decltype(out), CharT>;
+
+    std::basic_string<CharT> buffer{text, text + N};
+    // Note not too found of this hack
+    Str* data = reinterpret_cast<Str*>(const_cast<CharT*>(buffer.c_str()));
+
+    auto format_ctx = std::__format_context_create<decltype(out), CharT>(out, std::make_format_args<FormatCtxT>(*data));
+    formatter.format(*data, format_ctx);
+    assert(result == expected);
+  }
+
+  template <class CharT>
+  void test_termination_condition(const std::basic_string<CharT>& expected, const std::basic_string<CharT>& f) const {
+    // The format-spec is valid if completely consumed or terminates at a '}'.
+    // The valid inputs all end with a '}'. The test is executed twice:
+    // - first with the terminating '}',
+    // - second consuming the entire input.
+    std::basic_string_view<CharT> fmt{f};
+    assert(fmt.back() == CharT('}') && "Pre-condition failure");
+
+    test(expected, fmt);
+    fmt.remove_suffix(1);
+    test(expected, fmt);
+  }
+};
+
+template <Tester t, class CharT>
+void test_helper_wrapper(std::basic_string<CharT> expected, std::basic_string<CharT> fmt) {
+  t.test_termination_condition(expected, fmt);
+}
+
+template <class CharT>
+void test_array() {
+  test_helper_wrapper<" azAZ09,./<>?">(STR(" azAZ09,./<>?"), STR("}"));
+
+  std::basic_string<CharT> s(CSTR("abc\0abc"), 7);
+  test_helper_wrapper<"abc\0abc">(s, STR("}"));
+
+  test_helper_wrapper<"world">(STR("world"), STR("}"));
+  test_helper_wrapper<"world">(STR("world"), STR("_>}"));
+
+  test_helper_wrapper<"world">(STR("   world"), STR(">8}"));
+  test_helper_wrapper<"world">(STR("___world"), STR("_>8}"));
+  test_helper_wrapper<"world">(STR("_world__"), STR("_^8}"));
+  test_helper_wrapper<"world">(STR("world___"), STR("_<8}"));
+
+  test_helper_wrapper<"world">(STR("world"), STR(".5}"));
+  test_helper_wrapper<"universe">(STR("unive"), STR(".5}"));
+
+  test_helper_wrapper<"world">(STR("%world%"), STR("%^7.7}"));
+  test_helper_wrapper<"universe">(STR("univers"), STR("%^7.7}"));
+}
+
+int main(int, char**) {
+  test_array<char>();
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+  test_array<wchar_t>();
+#endif
+
+  return 0;
+}

libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/types.compile.pass.cpp

 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   // Tests the special formatter that converts a char to a wchar_t.
   assert_formatter_is_enabled<char, wchar_t>();
 #endif
   assert_formatter_is_enabled<CharT, CharT>();
 
   assert_formatter_is_enabled<CharT*, CharT>();
   assert_formatter_is_enabled<const CharT*, CharT>();
+  assert_formatter_is_enabled<CharT[42], CharT>();
   assert_formatter_is_enabled<const CharT[42], CharT>();
   assert_formatter_is_enabled<std::basic_string<CharT>, CharT>();
   assert_formatter_is_enabled<std::basic_string_view<CharT>, CharT>();
 
   assert_formatter_is_enabled<bool, CharT>();
 
   assert_formatter_is_enabled<signed char, CharT>();
   assert_formatter_is_enabled<signed short, CharT>();