[llvm] [ADT] Add C++17-style insert_or_assign for DenseMap (PR #94151)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 3 14:48:30 PDT 2024


================
@@ -499,6 +499,34 @@ TEST(DenseMapCustomTest, ReserveTest) {
   }
 }
 
+TEST(DenseMapCustomTest, InsertOrAssignTest) {
+  DenseMap<int, CountCopyAndMove> Map;
+  CountCopyAndMove::Copy = 0;
+  CountCopyAndMove::Move = 0;
+
+  CountCopyAndMove val1;
+  auto try0 = Map.insert_or_assign(0, val1);
+  EXPECT_TRUE(try0.second);
+  EXPECT_EQ(0, CountCopyAndMove::Move);
+  EXPECT_EQ(1, CountCopyAndMove::Copy);
+
+  auto try1 = Map.insert_or_assign(0, val1);
+  EXPECT_FALSE(try1.second);
+  EXPECT_EQ(0, CountCopyAndMove::Move);
+  EXPECT_EQ(2, CountCopyAndMove::Copy);
+
+  CountCopyAndMove val2;
+  auto try2 = Map.insert_or_assign(2, val2);
+  EXPECT_TRUE(try2.second);
+  EXPECT_EQ(0, CountCopyAndMove::Move);
+  EXPECT_EQ(3, CountCopyAndMove::Copy);
+
+  auto try3 = Map.insert_or_assign(2, std::move(val2));
+  EXPECT_FALSE(try3.second);
+  EXPECT_EQ(1, CountCopyAndMove::Move);
+  EXPECT_EQ(3, CountCopyAndMove::Copy);
+}
+
----------------
dwblaikie wrote:

The updated test doesn't look like it differentiates between ctor and assignment, though? I guess by virtue of it not being default constructible it necessarily can't be construct+assign in the cases of new values? That's a bit subtle, and I'd have thought testing that only copy/move construction happened on new insertion, and only copy/move assignment happened when insert_or_assigning over an existing value would be more clear about the expectations?

https://github.com/llvm/llvm-project/pull/94151


More information about the llvm-commits mailing list