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

via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 2 17:47:25 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);
+}
+
----------------
c8ef wrote:

> This test coverage only covers the rvalue ref Key overload, yeah? Be good to have similar coverage for the const ref overload too.

I have updated the test case to test both APIs.

> Also, some of the value/point of `insert_or_assign` is to directly construct the value where possible, and otherwise fallback to assignment. But this test doesn't differentiate between whether the object was default constructed then assigned to, or directly copy/move constructed. To get that more precise test coverage it might be necessary to add more detailed tracking to the CountCopyAndMove helper - but I think we might have a more reusable/generic/detailed type shared across more of these ADT unit tests that could be used?

I'm not completely sure how to test it as it appears to require integration with the DenseMap internal structure. Could you provide some insight on this?


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


More information about the llvm-commits mailing list