[PATCH] D67665: [ADT] Add 2 DenseMap::insert_or_assign overloads

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: include/llvm/ADT/DenseMap.h:206-231
+  template <typename V>
+  std::pair<iterator, bool> insert_or_assign(KeyT &&Key, V &&Val) {
+    BucketT *B;
+    bool Added = !LookupBucketFor(Key, B);
+    if (Added) {
+      B = InsertIntoBucket(B, std::move(Key), std::forward<V>(Val));
+    } else {
----------------
dblaikie wrote:
> Is this significantly cheaper than - say, try_emplace + assignment? (the latter seems like it'd keep the implementation simpler/less risky)
> 
>   auto {Iter, Inserted} = m.try_emplace(key, value);
>   if (!Inserted)
>     Iter->second = value
> 
> & at that point, should it perhaps be a utility, rather than reimplementing it in every container?
Thanks! I can do:

```
    BucketT *B;
    bool Added = !LookupBucketFor(Key, B);
    if (Added)
      B = InsertIntoBucket(B, std::move(Key), std::forward<V>(Val));
    else
      B->getSecond() = std::forward<V>(Val);
    return std::make_pair(makeIterator(B, getBucketsEnd(), *this, true), Added);
```

but that seems no better than:

```
    auto Ret = try_emplace(std::move(Key), std::forward<V>(Val));
    if (!Ret.second)
      Ret.first->second = std::forward<V>(Val);
    return Ret;
```

So I'll use the simple version.


================
Comment at: unittests/ADT/DenseMapTest.cpp:195
+TEST(DenseMapCustomTest, InsertOrAssignTest) {
+  DenseMap<int, std::unique_ptr<int>> Map;
+  auto Try1 = Map.insert_or_assign(0, new int(1));
----------------
dblaikie wrote:
> I'd probably test this with something simpler than std::unique_ptr - to make the test more targeted/clear as to what it's testing.
I agree it is less clear but it has a nice property. `std::unique_ptr<int>` as the value type can be used to detect memory leaks in a `-DLLVM_USE_SANITIZER=Address` build.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67665