[libcxx-commits] [PATCH] D98097: [libc++] "Merged wording" for D98077 and D96986

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__config:1462
+#if defined(__GNUC__) || defined(__clang__)
+#define _LIBCPP_FORMAT_PRINTF(...)                                             \
+  __attribute__((__format__(__printf__, __VA_ARGS__)))
----------------
Mordante wrote:
> Wouldn't it be better use 2 named arguments instead of `...`. The latter isn't officially part of C++98 and the attribute requires 3 arguments.
I had thought I was copying style from elsewhere in this file, but I guess only `_LIBCPP_DIAGNOSE_WARNING`/`_LIBCPP_DIAGNOSE_ERROR` actually use `...` for this purpose. So the only benefit I'm getting from `...` is that I don't have to worry about naming or whitespace conventions.
How do we feel about
```
#define _LIBCPP_FORMAT_PRINTF(a, b)   \
  __attribute__((__format__(__printf__, a, b)))
```
We already use `a, b` as macro parameter names on line 227. If this naming/whitespace looks acceptable, I don't mind making this change.

FWIW, I claim C++98-portability is not a concern, given that we already use `...` specifically in C++98 mode, e.g. in `#define static_assert(...)` on line 898.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98097