Skip to content

Commit

Permalink
[ADT] Fix MapVector when 'Map::mapped_type != unsigned'.
Browse files Browse the repository at this point in the history
Previously MapVector assumed `Map::mapped_type` was `unsigned`.
This caused problems when using MapVector with a user-specified
map where this didn't hold (For example StringMap<unsigned>).

This patch adjusts MapVector to use the same type as the underlying
map, avoiding reference binding errors in functions like `insert`.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@329523 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
EricWF committed Apr 8, 2018
1 parent 0edb3bf commit 16f832e
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
16 changes: 10 additions & 6 deletions include/llvm/ADT/MapVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ class MapVector {
MapType Map;
VectorType Vector;

static_assert(
std::is_integral<typename MapType::mapped_type>::value,
"The mapped_type of the specified Map must be an integral type");

public:
using value_type = typename VectorType::value_type;
using size_type = typename VectorType::size_type;
Expand Down Expand Up @@ -93,9 +97,9 @@ class MapVector {
}

ValueT &operator[](const KeyT &Key) {
std::pair<KeyT, unsigned> Pair = std::make_pair(Key, 0);
std::pair<KeyT, typename MapType::mapped_type> Pair = std::make_pair(Key, 0);
std::pair<typename MapType::iterator, bool> Result = Map.insert(Pair);
unsigned &I = Result.first->second;
auto &I = Result.first->second;
if (Result.second) {
Vector.push_back(std::make_pair(Key, ValueT()));
I = Vector.size() - 1;
Expand All @@ -112,9 +116,9 @@ class MapVector {
}

std::pair<iterator, bool> insert(const std::pair<KeyT, ValueT> &KV) {
std::pair<KeyT, unsigned> Pair = std::make_pair(KV.first, 0);
std::pair<KeyT, typename MapType::mapped_type> Pair = std::make_pair(KV.first, 0);
std::pair<typename MapType::iterator, bool> Result = Map.insert(Pair);
unsigned &I = Result.first->second;
auto &I = Result.first->second;
if (Result.second) {
Vector.push_back(std::make_pair(KV.first, KV.second));
I = Vector.size() - 1;
Expand All @@ -125,9 +129,9 @@ class MapVector {

std::pair<iterator, bool> insert(std::pair<KeyT, ValueT> &&KV) {
// Copy KV.first into the map, then move it into the vector.
std::pair<KeyT, unsigned> Pair = std::make_pair(KV.first, 0);
std::pair<KeyT, typename MapType::mapped_type> Pair = std::make_pair(KV.first, 0);
std::pair<typename MapType::iterator, bool> Result = Map.insert(Pair);
unsigned &I = Result.first->second;
auto &I = Result.first->second;
if (Result.second) {
Vector.push_back(std::move(KV));
I = Vector.size() - 1;
Expand Down
39 changes: 39 additions & 0 deletions unittests/ADT/MapVectorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,45 @@ TEST(MapVectorTest, NonCopyable) {
ASSERT_EQ(*MV.find(2)->second, 2);
}

template <class IntType> struct MapVectorMappedTypeTest : ::testing::Test {
using int_type = IntType;
};

using MapIntTypes = ::testing::Types<int, long, long long, unsigned,
unsigned long, unsigned long long>;
TYPED_TEST_CASE(MapVectorMappedTypeTest, MapIntTypes);

TYPED_TEST(MapVectorMappedTypeTest, DifferentDenseMap) {
// Test that using a map with a mapped type other than 'unsigned' compiles
// and works.
using IntType = typename TestFixture::int_type;
using MapVectorType = MapVector<int, int, DenseMap<int, IntType>>;

MapVectorType MV;
std::pair<typename MapVectorType::iterator, bool> R;

R = MV.insert(std::make_pair(1, 2));
ASSERT_EQ(R.first, MV.begin());
EXPECT_EQ(R.first->first, 1);
EXPECT_EQ(R.first->second, 2);
EXPECT_TRUE(R.second);

const std::pair<int, int> Elem(1, 3);
R = MV.insert(Elem);
ASSERT_EQ(R.first, MV.begin());
EXPECT_EQ(R.first->first, 1);
EXPECT_EQ(R.first->second, 2);
EXPECT_FALSE(R.second);

int& value = MV[4];
EXPECT_EQ(value, 0);
value = 5;

EXPECT_EQ(MV.size(), 2u);
EXPECT_EQ(MV[1], 2);
EXPECT_EQ(MV[4], 5);
}

TEST(SmallMapVectorSmallTest, insert_pop) {
SmallMapVector<int, int, 32> MV;
std::pair<SmallMapVector<int, int, 32>::iterator, bool> R;
Expand Down

0 comments on commit 16f832e

Please sign in to comment.