Skip to content

[ADT] Add DenseMap::insert_range #133600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 30, 2025
Merged

[ADT] Add DenseMap::insert_range #133600

merged 3 commits into from
Mar 30, 2025

Conversation

vbvictor
Copy link
Contributor

@vbvictor vbvictor commented Mar 29, 2025

This PR add DenseMap::insert_range to DenseMap for consistency with existing DenseSet::insert_range, SmallSet::insert_range and std::map::insert_range.

@llvmbot
Copy link
Member

llvmbot commented Mar 29, 2025

@llvm/pr-subscribers-llvm-adt

Author: Baranov Victor (vbvictor)

Changes

This PR add DenseMap::insert_range to DenseMap for consistency with existing DenseSet::insert_range and SmallSet::insert_range.


Full diff: https://github.com/llvm/llvm-project/pull/133600.diff

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/DenseMap.h (+5)
  • (modified) llvm/unittests/ADT/DenseMapTest.cpp (+11)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index f0f992f8eac38..52834c8528eb0 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_ADT_DENSEMAP_H
 #define LLVM_ADT_DENSEMAP_H
 
+#include "llvm/ADT/ADL.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/EpochTracker.h"
 #include "llvm/Support/AlignOf.h"
@@ -302,6 +303,10 @@ class DenseMapBase : public DebugEpochBase {
       insert(*I);
   }
 
+  template <typename Range> void insert_range(Range &&R) {
+    insert(adl_begin(R), adl_end(R));
+  }
+
   template <typename V>
   std::pair<iterator, bool> insert_or_assign(const KeyT &Key, V &&Val) {
     auto Ret = try_emplace(Key, std::forward<V>(Val));
diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp
index d1bbdde8dfc26..695ac99728a02 100644
--- a/llvm/unittests/ADT/DenseMapTest.cpp
+++ b/llvm/unittests/ADT/DenseMapTest.cpp
@@ -379,6 +379,17 @@ TEST(DenseMapCustomTest, EqualityComparison) {
   EXPECT_NE(M1, M3);
 }
 
+TEST(DenseMapCustomTest, InsertRange) {
+  DenseMap<int, int> M;
+
+  std::pair<int, int> InputVals[3] = {{0, 0}, {0, 1}, {1, 2}};
+  M.insert_range(InputVals);
+
+  EXPECT_EQ(2u, M.size());
+  EXPECT_THAT(M, testing::UnorderedElementsAre(testing::Pair(0, 0),
+                                               testing::Pair(1, 2)));
+}
+
 // Test for the default minimum size of a DenseMap
 TEST(DenseMapCustomTest, DefaultMinReservedSizeTest) {
   // IF THIS VALUE CHANGE, please update InitialSizeTest, InitFromIterator, and

@@ -379,6 +379,17 @@ TEST(DenseMapCustomTest, EqualityComparison) {
EXPECT_NE(M1, M3);
}

TEST(DenseMapCustomTest, InsertRange) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also have a test for SmallDenseSet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in llvm/unittests/ADT/DenseSetTest.cpp

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant SmallDenseMap

Copy link
Contributor Author

@vbvictor vbvictor Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added SmallDenseMap, should I leave test for SmallDenseSet as is because it was absent in DenseSetTest.cpp?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think it's fine either way.

@kuhar kuhar requested a review from kazutakahirata March 29, 2025 22:48
@kazutakahirata
Copy link
Contributor

@vbvictor I don't have any more comments beyond what @kuhar already mentioned, but I just want to say thanks for the PR!

@vbvictor
Copy link
Contributor Author

If you are happy with the changes, you could merge the PR when all checks are passed.
I don't have write access.

@@ -379,6 +379,17 @@ TEST(DenseMapCustomTest, EqualityComparison) {
EXPECT_NE(M1, M3);
}

TEST(DenseMapCustomTest, InsertRange) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think it's fine either way.

@kuhar kuhar merged commit 8ecb2f9 into llvm:main Mar 30, 2025
11 checks passed
SchrodingerZhu pushed a commit to SchrodingerZhu/llvm-project that referenced this pull request Mar 31, 2025
This PR add `DenseMap::insert_range` to `DenseMap` for consistency with
existing `DenseSet::insert_range`, `SmallSet::insert_range` and
`std::map::insert_range`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants