Apply STLs review comments

microsoft · Jun 22, 2021 · a9e8c3d · a9e8c3d
1 parent 0cd5770
commit a9e8c3d
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 39 deletions.
diff --git a/stl/inc/queue b/stl/inc/queue
@@ -271,32 +271,34 @@ public:
             is_nothrow_move_constructible_v<value_compare>) // strengthened
         : c(_STD move(_Right.c), _Al), comp(_STD move(_Right.comp)) {}
 
-#if _HAS_CXX20
-    template <class _InIt, class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
-    priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Container& _Cont, const _Alloc& _Al)
-        : c(_Cont, _Al), comp(_Pred) {
-        c.insert(c.end(), _First, _Last);
-        _STD make_heap(c.begin(), c.end(), comp);
-    }
-
-    template <class _InIt, class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
+    template <class _InIt, class _Alloc,
+        enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Alloc& _Al) : c(_First, _Last, _Al), comp() {
         _STD make_heap(c.begin(), c.end(), comp);
     }
 
-    template <class _InIt, class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
+    template <class _InIt, class _Alloc,
+        enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Alloc& _Al)
         : c(_First, _Last, _Al), comp(_Pred) {
         _STD make_heap(c.begin(), c.end(), comp);
     }
 
-    template <class _InIt, class _Alloc, enable_if_t<uses_allocator_v<_Container, _Alloc>, int> = 0>
+    template <class _InIt, class _Alloc,
+        enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
+    priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, const _Container& _Cont, const _Alloc& _Al)
+        : c(_Cont, _Al), comp(_Pred) {
+        c.insert(c.end(), _First, _Last);
+        _STD make_heap(c.begin(), c.end(), comp);
+    }
+
+    template <class _InIt, class _Alloc,
+        enable_if_t<_Is_iterator_v<_InIt> && uses_allocator_v<_Container, _Alloc>, int> = 0>
     priority_queue(_InIt _First, _InIt _Last, const _Pr& _Pred, _Container&& _Cont, const _Alloc& _Al)
         : c(_STD move(_Cont), _Al), comp(_Pred) {
         c.insert(c.end(), _First, _Last);
         _STD make_heap(c.begin(), c.end(), comp);
     }
-#endif // _HAS_CXX20
 
     _NODISCARD bool empty() const noexcept(noexcept(c.empty())) /* strengthened */ {
         return c.empty();
@@ -358,23 +360,20 @@ template <class _Pr, class _Container, class _Alloc,
                     uses_allocator<_Container, _Alloc>>,
         int> = 0>
 priority_queue(_Pr, _Container, _Alloc) -> priority_queue<typename _Container::value_type, _Container, _Pr>;
-#endif // _HAS_CXX17
 
-#if _HAS_CXX20
-template <class _Iter, class _Alloc, enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>>, int> = 0>
-priority_queue(_Iter, _Iter, _Alloc)
-    -> priority_queue<_Iter_value_t<_Iter>, vector<_Iter_value_t<_Iter>, _Alloc>, less<_Iter_value_t<_Iter>>>;
-
-template <class _Iter, class Compare, class _Alloc,
-    enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>>, int> = 0>
-priority_queue(_Iter, _Iter, Compare, _Alloc)
-    -> priority_queue<_Iter_value_t<_Iter>, vector<_Iter_value_t<_Iter>, _Alloc>, Compare>;
-
-template <class _Iter, class Compare, class Container, class _Alloc,
-    enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>>, int> = 0>
-priority_queue(_Iter, _Iter, Compare, Container, _Alloc)
-    -> priority_queue<typename Container::value_type, Container, Compare>;
-#endif // _HAS_CXX20
+template <class _Iter, class _Alloc, class _Container = vector<_Iter_value_t<_Iter>, _Alloc>,
+    enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>, uses_allocator<_Container, _Alloc>>, int> = 0>
+priority_queue(_Iter, _Iter, _Alloc) -> priority_queue<_Iter_value_t<_Iter>, _Container, less<_Iter_value_t<_Iter>>>;
+
+template <class _Iter, class _Compare, class _Alloc, class _Container = vector<_Iter_value_t<_Iter>, _Alloc>,
+    enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>, uses_allocator<_Container, _Alloc>>, int> = 0>
+priority_queue(_Iter, _Iter, _Compare, _Alloc) -> priority_queue<_Iter_value_t<_Iter>, _Container, _Compare>;
+
+template <class _Iter, class _Compare, class _Container, class _Alloc,
+    enable_if_t<conjunction_v<_Is_iterator<_Iter>, _Is_allocator<_Alloc>, uses_allocator<_Container, _Alloc>>, int> = 0>
+priority_queue(_Iter, _Iter, _Compare, _Container, _Alloc)
+    -> priority_queue<typename _Container::value_type, _Container, _Compare>;
+#endif // _HAS_CXX17
 
 template <class _Ty, class _Container, class _Pr,
     enable_if_t<_Is_swappable<_Container>::value && _Is_swappable<_Pr>::value, int> = 0>

diff --git a/tests/std/tests/P1425R4_queue_stack_constructors/env.lst b/tests/std/tests/P1425R4_queue_stack_constructors/env.lst
@@ -1,4 +1,4 @@
 # Copyright (c) Microsoft Corporation.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-RUNALL_INCLUDE ..\usual_20_matrix.lst
+RUNALL_INCLUDE ..\usual_matrix.lst
diff --git a/tests/std/tests/P1425R4_queue_stack_constructors/test.cpp b/tests/std/tests/P1425R4_queue_stack_constructors/test.cpp
@@ -34,10 +34,17 @@ struct custom_allocator {
         allocator<T>{}.deallocate(p, n);
     }
 
-    template <class... Args>
-    void construct(T* const p, Args&&... args) {
-        construct_at(p, forward<Args>(args)...);
+    template <class U>
+    bool operator==(const custom_allocator<U>&) noexcept {
+        return is_same_v<U, T>;
     }
+
+#if !_HAS_CXX20
+    template <class U>
+    bool operator!=(const custom_allocator<U>&) noexcept {
+        return !is_same_v<U, T>;
+    }
+#endif // !_HAS_CXX20
 };
 
 template <class Range>
@@ -83,66 +90,100 @@ void test_container() {
     }
 #endif // _HAS_CXX23
 
+#if _HAS_CXX17
     priority_queue pq1(range.begin(), range.end());
-    static_assert(is_same_v<decltype(pq1), priority_queue<int, vector<int, allocator<int>>, less<int>>>);
+    static_assert(is_same_v<decltype(pq1), priority_queue<int, vector<int>, less<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int>, less<int>> pq1(range.begin(), range.end());
+#endif // !_HAS_CXX17
     assert(pq1.size() == size(some_data));
     result = 5;
     while (!pq1.empty()) {
         assert(pq1.top() == result--);
         pq1.pop();
     }
 
+#if _HAS_CXX17
     priority_queue pq2(range.begin(), range.end(), custom_allocator<int>{});
     static_assert(is_same_v<decltype(pq2), priority_queue<int, vector<int, custom_allocator<int>>, less<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int, custom_allocator<int>>, less<int>> pq2(
+        range.begin(), range.end(), custom_allocator<int>{});
+#endif // !_HAS_CXX17
     assert(pq2.size() == size(some_data));
     result = 5;
     while (!pq2.empty()) {
         assert(pq2.top() == result--);
         pq2.pop();
     }
 
+#if _HAS_CXX17
     priority_queue pq3(range.begin(), range.end(), greater<int>{}, custom_allocator<int>{});
     static_assert(is_same_v<decltype(pq3), priority_queue<int, vector<int, custom_allocator<int>>, greater<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int, custom_allocator<int>>, greater<int>> pq3(
+        range.begin(), range.end(), greater<int>{}, custom_allocator<int>{});
+#endif // !_HAS_CXX17
     assert(pq3.size() == size(some_data));
     result = 0;
     while (!pq3.empty()) {
         assert(pq3.top() == result++);
         pq3.pop();
     }
 
-    vector cont(begin(additional_data), end(additional_data));
+    vector<int> cont(begin(additional_data), end(additional_data));
+#if _HAS_CXX17
     priority_queue pq4(range.begin(), range.end(), greater<int>{}, cont);
-    static_assert(is_same_v<decltype(pq4), priority_queue<int, vector<int, allocator<int>>, greater<int>>>);
+    static_assert(is_same_v<decltype(pq4), priority_queue<int, vector<int>, greater<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int>, greater<int>> pq4(range.begin(), range.end(), greater<int>{}, cont);
+#endif // !_HAS_CXX17
     assert(pq4.size() == size(some_data) + size(additional_data));
     result = 0;
     while (!pq4.empty()) {
         assert(pq4.top() == result++);
         pq4.pop();
     }
 
-    priority_queue pq5(range.begin(), range.end(), greater<int>{},
-        vector<int, allocator<int>>{begin(additional_data), end(additional_data)});
-    static_assert(is_same_v<decltype(pq5), priority_queue<int, vector<int, allocator<int>>, greater<int>>>);
+#if _HAS_CXX17
+    priority_queue pq5(
+        range.begin(), range.end(), greater<int>{}, vector<int>{begin(additional_data), end(additional_data)});
+    static_assert(is_same_v<decltype(pq5), priority_queue<int, vector<int>, greater<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int>, greater<int>> pq5(
+        range.begin(), range.end(), greater<int>{}, vector<int>{begin(additional_data), end(additional_data)});
+#endif // !_HAS_CXX17
     assert(pq5.size() == size(some_data) + size(additional_data));
     result = 0;
     while (!pq5.empty()) {
         assert(pq5.top() == result++);
         pq5.pop();
     }
 
-    vector cont2(begin(additional_data), end(additional_data), custom_allocator<int>{});
+    vector<int, custom_allocator<int>> cont2(begin(additional_data), end(additional_data), custom_allocator<int>{});
+#if _HAS_CXX17
     priority_queue pq6(range.begin(), range.end(), greater<int>{}, cont2, custom_allocator<int>{});
     static_assert(is_same_v<decltype(pq6), priority_queue<int, vector<int, custom_allocator<int>>, greater<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int, custom_allocator<int>>, greater<int>> pq6(
+        range.begin(), range.end(), greater<int>{}, cont2, custom_allocator<int>{});
+#endif // !_HAS_CXX17
     assert(pq6.size() == size(some_data) + size(additional_data));
     result = 0;
     while (!pq6.empty()) {
         assert(pq6.top() == result++);
         pq6.pop();
     }
 
+#if _HAS_CXX17
     priority_queue pq7(range.begin(), range.end(), greater<int>{},
         vector<int, custom_allocator<int>>{begin(additional_data), end(additional_data)}, custom_allocator<int>{});
     static_assert(is_same_v<decltype(pq7), priority_queue<int, vector<int, custom_allocator<int>>, greater<int>>>);
+#else // ^^^_HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
+    priority_queue<int, vector<int, custom_allocator<int>>, greater<int>> pq7(range.begin(), range.end(),
+        greater<int>{}, vector<int, custom_allocator<int>>{begin(additional_data), end(additional_data)},
+        custom_allocator<int>{});
+#endif // !_HAS_CXX17
     assert(pq7.size() == size(some_data) + size(additional_data));
     result = 0;
     while (!pq7.empty()) {