[libcxx-commits] [PATCH] D131868: [libc++][NFC] Refactor enable_ifs in vector

[Hui via Phabricator via libcxx-commits]

huixie90 accepted this revision as: huixie90.
huixie90 added a comment.

LGTM with comments addressed. lots of `enable_if`s are checking the cpp 17 forward iterator. I wonder if we can relax it to cpp 20 forward iterator. (cpp 17 forward iterator requires `reference` being a reference type, which is not important for these vector members IIUC)



================
Comment at: libcxx/include/vector:1003
+template <class _ForwardIterator, class>
+_LIBCPP_CONSTEXPR_AFTER_CXX17 void
 vector<_Tp, _Allocator>::__construct_at_end(_ForwardIterator __first, _ForwardIterator __last, size_type __n)
----------------
Mordante wrote:
> Here you removed and enable_if, is that really intended?
> I'm a bit concerned the CI passes with this change.
Technically the `enable_if` still applies since the default template argument with enable_if already exists in the function declaration few hundred lines above. But I agree it is not obvious since the declaration and definition are few hundred lines apart from each other.

Also I agree that using non-type template is better because it is consistent with others, although technically it is not required to use non-type template parameter since there is only one overload


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131868