[libcxx-commits] [PATCH] D93593: [libc++][format] Add __format_arg_store.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante planned changes to this revision.
Mordante marked 2 inline comments as done.
Mordante added a comment.

In D93593#2714591 <https://reviews.llvm.org/D93593#2714591>, @tschuett wrote:

> Funny enough there is [[deprecated("message")]], but there is no [[unstable("message")]].

:-)

For the CI passes. Still need to look at the attributes. I'll also look at polishing the code a bit further.



================
Comment at: libcxx/include/format:365-370
+template <class _Visitor, class _Context>
+_LIBCPP_INLINE_VISIBILITY auto
+visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg)
+    -> decltype(_VSTD::visit(forward<_Visitor>(__vis), __arg.__value_)) {
+  return _VSTD::visit(forward<_Visitor>(__vis), __arg.__value_);
+}
----------------
Quuxplusone wrote:
> Nit throughout: `_VSTD::forward` (and all other functions of >=1 argument) because ADL.
> But you //don't// need (and therefore shouldn't put) `_VSTD::` on concepts, classes, or variables; i.e. you should write `monostate` not `_VSTD::monostate`, and `same_as` not `_VSTD::same_as`, and so on.
> Essentially, just as you should think of `explicit` as meaning "Here comes a constructor!", you should think of `_VSTD::` in the libc++ codebase as meaning "Here comes a function call!"
> 
> Hopefully this fixes line 442's `)&&sizeof(_Tp)` misformatting, as well.
Good point. I addressed these issues.
The formatting is still not great. Guess we need to wait for clang-format to properly implement concepts formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93593