[libcxx-commits] [PATCH] D127570: [libc++][format] Use forwarding references.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 2 inline comments as done.
Mordante added inline comments.


================
Comment at: libcxx/include/__format/format_arg.h:260
 
-  _LIBCPP_HIDE_FROM_ABI explicit handle(typename __basic_format_arg_value<_Context>::__handle __handle) noexcept
+  _LIBCPP_HIDE_FROM_ABI explicit handle(typename __basic_format_arg_value<_Context>::__handle& __handle) noexcept
       : __handle_(__handle) {}
----------------
ldionne wrote:
> IIUC this means that we can't construct a `handle` from a temporary anymore, however I don't think I saw any tests that needed to change in reaction to that -- is that expected?
We can still construct a `handle` using a temporary, there are test for that. However the temporary is "stored" in `__basic_format_arg_value<_Context>::__handle` and not in this object.


================
Comment at: libcxx/test/std/utilities/format/format.arguments/format.arg.store/make_format_args.pass.cpp:27-28
 
-  std::make_format_args(42, nullptr, false, 1.0);
+#ifdef _LIBCPP_VERSION
+  static_assert(
+      std::same_as<decltype(store), std::__format_arg_store<std::format_context, int, nullptr_t, bool, double>>);
----------------
ldionne wrote:
> You could also use `LIBCPP_STATIC_ASSERT` if you prefer.
Good one! I forgot about that macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127570