[libcxx-commits] [PATCH] D121138: [libc++][format] Adds formatter<charT[N], charT>.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__format/formatter_string.h:128
 
   _LIBCPP_HIDE_FROM_ABI auto format(const _CharT __str[_Size], auto& __ctx)
       -> decltype(__ctx.out()) {
----------------
Mordante wrote:
> Quuxplusone wrote:
> > 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.)
> 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`.
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121138/new/

https://reviews.llvm.org/D121138