[libcxx-commits] [PATCH] D145800: [libc++] Granularizes vector.

Eric Fiselier via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Mar 20 12:29:01 PDT 2023


EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: mikhail.ramalho.

I think this is a really bad idea. 
Splitting the specialization out so that only some files can depend on it, while others cannot, allowing some bits of the code to only see a partial implementation is going to be bug prone.

I'm also not sure I agree with the bug. The standard only requires the declaration of the type to be available without the format include. It says nothing about requiring the type to be complete IMHO.
Like that `hash<std::vector<bool>>` is declared in `<vector>` but technically you need to include `<utility>` to get its definition.

This dance is done specifically so that anytime a type T with a hash specialization is seen by the user, `hash<T>` has been properly declared, so the compiler knows to not instantiate the default implementation.

I would like to see other alternative solutions explored. I don't think this one should proceed.



================
Comment at: libcxx/include/__vector/vector.h:339
 
-#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
-#  pragma GCC system_header
----------------
Where did this go?


================
Comment at: libcxx/include/__vector/vector.h:3351
-_LIBCPP_BEGIN_NAMESPACE_STD
-namespace pmr {
-template <class _ValueT>
----------------
Where did this go?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145800



More information about the libcxx-commits mailing list