[libcxx-commits] [PATCH] D131898: [libc++] Implement P0591R4 (Utility functions to implement uses-allocator construction)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__memory/uses_allocator_construction.h:171
+_LIBCPP_HIDE_FROM_ABI constexpr auto __make_obj_using_allocator(const _Alloc& __alloc, _Args&&... __args)
+    -> decltype(std::make_from_tuple<_Type>(
+        std::__uses_allocator_construction_args<_Type>(__alloc, std::forward<_Args>(__args)...))) {
----------------
huixie90 wrote:
> In your original version , this is doing sfinae, but now you’ve changed the use_allocator_constructution_args to static_assert in the function body.  Does this still do the things you want?
Yes. The instantiation error is actually requested by the standard, so this fails correctly.


================
Comment at: libcxx/include/__memory/uses_allocator_construction.h:131
+template <class _Tp>
+constexpr bool __convertible_to_const_pair_ref =
+    decltype(__detail::__convertible_to_const_pair_ref_impl<_Tp>(0))::value;
----------------
huixie90 wrote:
> huixie90 wrote:
> > `inline`
> question : do we need hidden from abi macro for inline constexpr variables 
We don't do it anywhere else and it would also kind-of defeat the purpose of marking it `inline`. I don't think it will ever be emitted anyways, since AFAIK the only time clang will emit the symbol is when you take the address of it, which we will never do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131898