[libcxx-commits] [PATCH] D58582: Implement P0433: deduction guides for <set>

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

Almost there. A couple nits, a few `const`s, and landing D58587 <https://reviews.llvm.org/D58587>, and we're done.



================
Comment at: include/set:862
+set(_InputIterator, _InputIterator, _Allocator)
+  -> set<typename iterator_traits<_InputIterator>::value_type,
+         less<typename iterator_traits<_InputIterator>::value_type>, _Allocator>;
----------------
Any reason that you're not using `__iter_value_type` here? (Besides that it is declared in D58587)


================
Comment at: include/set:1365
+template<class _InputIterator,
+         class _Compare = less<typename iterator_traits<_InputIterator>::value_type>,
+         class _Allocator = allocator<typename iterator_traits<_InputIterator>::value_type>,
----------------
same comment re: `__iter_value_type`


================
Comment at: test/std/containers/associative/set/set.cons/deduct.pass.cpp:94
+    std::set<long> source;
+    std::set s(source, std::allocator<int>());
+    ASSERT_SAME_TYPE(decltype(s), std::set<long>);
----------------
Please change to `std::allocator<long>`  When we tighten up the constraints on set, it won't break this test.



Repository:
  rCXX libc++

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

https://reviews.llvm.org/D58582