[libcxx-commits] [PATCH] D115977: [libc++] Implement P1425R4 (Iterator pair constructors for std::stack and std::queue)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/queue:277
+    template <class _InputIterator, class _Alloc>
+    _LIBCPP_HIDE_FROM_ABI queue(_InputIterator __first, _InputIterator __second, const _Alloc& __alloc)
+        : c(__first, __second, __alloc) {}
----------------
Quuxplusone wrote:
> Does this overload need `, __enable_if_t<uses_allocator<container_type, _Alloc>::value>* = 0` similar to the below overloads? I suspect it does. (And then please add a regression test. The regression test will use SFINAE but not CTAD.)
Actually even (hand-rolled) SFINAE is unnecessary. A regression test for this could be as simple as
```
static_assert(!std::is_constructible_v<std::queue<int>, int, int, std::allocator<int>>);
static_assert(!std::is_constructible_v<std::queue<int>, int*, int*, int>);
static_assert( std::is_constructible_v<std::queue<int, std::deque<int, test_allocator<int>>>, int, int, test_allocator<int>>);
static_assert(!std::is_constructible_v<std::queue<int, std::deque<int, test_allocator<int>>>, int, int, std::allocator<int>>);
```
and likewise for `stack`. (Preemptive whitespace advice: These lines //are// long, but I wouldn't break them.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115977