[PATCH] D10669: Part 1 of N4279 - "Improved insertion interface for unique-key maps"

[Eric Fiselier]

EricWF accepted this revision.

This revision is now accepted and ready to land.

LGTM modulo inline comments.


================
Comment at: include/map:1138
@@ +1137,3 @@
+        else
+            return emplace_hint(__p, 
+                      _VSTD::piecewise_construct, _VSTD::forward_as_tuple(__k), 
----------------
mclow.lists wrote:
> I think this should be `__h`, not `__p`, since `__p == end()`
I actually think you should leave this as is. `__tree` seems to properly handle the end iterator as a hint. `__tree::__find_equal(hint, ...)` assumes that if `hint` is `end()` then the value should be inserted just prior to `hint`. 

You should be able to implement the the `try_emplace(hint, ...)` methods in terms of the normal `try_emplace` methods because we ignore the hint anyway.





================
Comment at: test/std/containers/associative/map/map.modifiers/try.emplace.pass.cpp:70
@@ +69,3 @@
+        for ( int i = 0; i < 20; i += 2 )
+            m.emplace ( i, Moveable(i, (double) i));
+        assert(m.size() == 10);
----------------
Why not use `try_emplace` here as well? 

================
Comment at: test/std/containers/associative/map/map.modifiers/try.emplace.pass.cpp:73
@@ +72,3 @@
+
+        Moveable mv1(3, 3.0);
+        r = m.try_emplace(2, std::move(mv1));
----------------
For the sake of covering corner cases could we test every inserted value? This applies for `insert_or_assign` as well.

```
for (int i=0; i < 20; i += 2) {
  Movable mv1(i, (double)i);
  r = m.try_emplace(2, std::move(mv1));
  assert(m.size() == 10);
  assert(!r.second);                  // was not inserted
  assert(!mv1.moved());               // was not moved from
  assert(r.first->first == 2);        // key
}
```

================
Comment at: test/std/containers/associative/map/map.modifiers/try.emplace.pass.cpp:80
@@ +79,3 @@
+
+        r = m.try_emplace(3, std::move(mv1));
+        assert(m.size() == 11);
----------------
We should test the cases where insertion happens at the front and at the back.


http://reviews.llvm.org/D10669