[libcxx-commits] [PATCH] D58587: Implement P0433: deduction guides for <map>

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

This looks good to me, with a few nits.



================
Comment at: include/map:2132
+multimap(initializer_list<pair<_Key, _Tp>>, _Compare = _Compare(), _Allocator = _Allocator())
+  -> multimap<typename _VSTD::remove_const<_Key>::type, _Tp, _Compare, _Allocator>;
+
----------------
You're very consistent here with qualifying `remove_const`, and nothing else. Why?


================
Comment at: test/std/containers/associative/map/map.cons/deduct.fail.cpp:62
+    }
+    {
+        // refuse to rebind the allocator if Allocator::value_type is not exactly what we expect
----------------
Why is this test commented out?


================
Comment at: test/std/containers/associative/map/map.cons/deduct.pass.cpp:87
+    std::map<int, long> source;
+    std::map m(source, std::allocator<int>());
+    ASSERT_SAME_TYPE(decltype(m), decltype(source));
----------------
This works? I would think that it should not. We rebind the allocator internally into something else, but we should probably reject this. [But not in this patch]


================
Comment at: test/std/containers/associative/multimap/multimap.cons/deduct.fail.cpp:60
+    {
+        // refuse to rebind the allocator if Allocator::value_type is not exactly what we expect
+        // std::pair<int, int> arr[] = { {1,1}, {2,2}, {3,3} };
----------------
Quuxplusone wrote:
> mclow.lists wrote:
> > What's going on here?
> > Does this test need to be uncommented? or removed?
> The reason it's commented out is that the error message that's actually printed is a hard error from inside `include/map` and I don't know how to write an expected-error line for that. Is that possible? Here's the error as printed:
> ```
> File $ROOT/llvm/projects/libcxx/include/map Line 1671: static_assert failed due to requirement 'is_same<std::__1::pair<int, long>, std::__1::pair<const int, long> >::value' "Allocator::value_type must be same type as value_type"
> ```
See test/std/algorithms/alg.modifying.operations/alg.random.sample/sample.fail.cpp for an example.
Basically, you would write:
```
// expected-error-re@ map:* {{static_assert failed{{( due to requirement '.*')?}} "Allocator::value_type must be same type as value_type"}}
```



Repository:
  rCXX libc++

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

https://reviews.llvm.org/D58587