[llvm] r211328 - Revert "Add StringMap::insert(pair) consistent with the standard associative container concept."
Rafael Espindola
rafael.espindola at gmail.com
Thu Jun 19 17:23:04 PDT 2014
Author: rafael
Date: Thu Jun 19 19:23:03 2014
New Revision: 211328
URL: http://llvm.org/viewvc/llvm-project?rev=211328&view=rev
Log:
Revert "Add StringMap::insert(pair) consistent with the standard associative container concept."
This reverts commit r211309.
It looks like it broke some bots:
http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/15563/steps/compile/logs/stdio
Modified:
llvm/trunk/include/llvm/ADT/StringMap.h
llvm/trunk/lib/Support/StringMap.cpp
llvm/trunk/unittests/ADT/StringMapTest.cpp
Modified: llvm/trunk/include/llvm/ADT/StringMap.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringMap.h?rev=211328&r1=211327&r2=211328&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/StringMap.h (original)
+++ llvm/trunk/include/llvm/ADT/StringMap.h Thu Jun 19 19:23:03 2014
@@ -64,7 +64,7 @@ protected:
}
StringMapImpl(unsigned InitSize, unsigned ItemSize);
- unsigned RehashTable(unsigned BucketNo = 0);
+ void RehashTable();
/// LookupBucketFor - Look up the bucket that the specified string should end
/// up in. If it already exists as a key in the map, the Item pointer for the
@@ -323,28 +323,6 @@ public:
return true;
}
- /// insert - Inserts the specified key/value pair into the map if the key
- /// isn't already in the map. The bool component of the returned pair is true
- /// if and only if the insertion takes place, and the iterator component of
- /// the pair points to the element with key equivalent to the key of the pair.
- std::pair<iterator, bool> insert(std::pair<StringRef, ValueTy> KV) {
- unsigned BucketNo = LookupBucketFor(KV.first);
- StringMapEntryBase *&Bucket = TheTable[BucketNo];
- if (Bucket && Bucket != getTombstoneVal())
- return std::make_pair(iterator(TheTable + BucketNo, false),
- false); // Already exists in map.
-
- if (Bucket == getTombstoneVal())
- --NumTombstones;
- Bucket =
- MapEntryTy::Create(KV.first, Allocator, std::move(KV.second));
- ++NumItems;
- assert(NumItems + NumTombstones <= NumBuckets);
-
- BucketNo = RehashTable(BucketNo);
- return std::make_pair(iterator(TheTable + BucketNo, false), true);
- }
-
// clear - Empties out the StringMap
void clear() {
if (empty()) return;
@@ -368,7 +346,24 @@ public:
/// return.
template <typename InitTy>
MapEntryTy &GetOrCreateValue(StringRef Key, InitTy Val) {
- return *insert(std::make_pair(Key, std::move(Val))).first;
+ unsigned BucketNo = LookupBucketFor(Key);
+ StringMapEntryBase *&Bucket = TheTable[BucketNo];
+ if (Bucket && Bucket != getTombstoneVal())
+ return *static_cast<MapEntryTy*>(Bucket);
+
+ MapEntryTy *NewItem = MapEntryTy::Create(Key, Allocator, std::move(Val));
+
+ if (Bucket == getTombstoneVal())
+ --NumTombstones;
+ ++NumItems;
+ assert(NumItems + NumTombstones <= NumBuckets);
+
+ // Fill in the bucket for the hash table. The FullHashValue was already
+ // filled in by LookupBucketFor.
+ Bucket = NewItem;
+
+ RehashTable();
+ return *NewItem;
}
MapEntryTy &GetOrCreateValue(StringRef Key) {
Modified: llvm/trunk/lib/Support/StringMap.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/StringMap.cpp?rev=211328&r1=211327&r2=211328&view=diff
==============================================================================
--- llvm/trunk/lib/Support/StringMap.cpp (original)
+++ llvm/trunk/lib/Support/StringMap.cpp Thu Jun 19 19:23:03 2014
@@ -181,7 +181,7 @@ StringMapEntryBase *StringMapImpl::Remov
/// RehashTable - Grow the table, redistributing values into the buckets with
/// the appropriate mod-of-hashtable-size.
-unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
+void StringMapImpl::RehashTable() {
unsigned NewSize;
unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
@@ -193,10 +193,9 @@ unsigned StringMapImpl::RehashTable(unsi
} else if (NumBuckets-(NumItems+NumTombstones) <= NumBuckets/8) {
NewSize = NumBuckets;
} else {
- return BucketNo;
+ return;
}
- unsigned NewBucketNo = BucketNo;
// Allocate one extra bucket which will always be non-empty. This allows the
// iterators to stop at end.
StringMapEntryBase **NewTableArray =
@@ -216,8 +215,6 @@ unsigned StringMapImpl::RehashTable(unsi
if (!NewTableArray[NewBucket]) {
NewTableArray[FullHash & (NewSize-1)] = Bucket;
NewHashArray[FullHash & (NewSize-1)] = FullHash;
- if (I == BucketNo)
- NewBucketNo = NewBucket;
continue;
}
@@ -230,8 +227,6 @@ unsigned StringMapImpl::RehashTable(unsi
// Finally found a slot. Fill it in.
NewTableArray[NewBucket] = Bucket;
NewHashArray[NewBucket] = FullHash;
- if (I == BucketNo)
- NewBucketNo = NewBucket;
}
}
@@ -240,5 +235,4 @@ unsigned StringMapImpl::RehashTable(unsi
TheTable = NewTableArray;
NumBuckets = NewSize;
NumTombstones = 0;
- return NewBucketNo;
}
Modified: llvm/trunk/unittests/ADT/StringMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=211328&r1=211327&r2=211328&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/StringMapTest.cpp (original)
+++ llvm/trunk/unittests/ADT/StringMapTest.cpp Thu Jun 19 19:23:03 2014
@@ -10,7 +10,6 @@
#include "gtest/gtest.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/DataTypes.h"
-#include <tuple>
using namespace llvm;
namespace {
@@ -204,43 +203,6 @@ TEST_F(StringMapTest, InsertTest) {
assertSingleItemMap();
}
-// Test insert(pair<K, V>) method
-TEST_F(StringMapTest, InsertPairTest) {
- bool Inserted;
- StringMap<uint32_t>::iterator NewIt;
- std::tie(NewIt, Inserted) =
- testMap.insert(std::make_pair(testKeyFirst, testValue));
- EXPECT_EQ(1u, testMap.size());
- EXPECT_EQ(testValue, testMap[testKeyFirst]);
- EXPECT_EQ(testKeyFirst, NewIt->first());
- EXPECT_EQ(testValue, NewIt->second);
- EXPECT_TRUE(Inserted);
-
- StringMap<uint32_t>::iterator ExistingIt;
- std::tie(ExistingIt, Inserted) =
- testMap.insert(std::make_pair(testKeyFirst, testValue + 1));
- EXPECT_EQ(1u, testMap.size());
- EXPECT_EQ(testValue, testMap[testKeyFirst]);
- EXPECT_FALSE(Inserted);
- EXPECT_EQ(NewIt, ExistingIt);
-}
-
-// Test insert(pair<K, V>) method when rehashing occurs
-TEST_F(StringMapTest, InsertRehashingPairTest) {
- // Check that the correct iterator is returned when the inserted element is
- // moved to a different bucket during internal rehashing. This depends on
- // the particular key, and the implementation of StringMap and HashString.
- // Changes to those might result in this test not actually checking that.
- StringMap<uint32_t> t(1);
- EXPECT_EQ(1u, t.getNumBuckets());
-
- StringMap<uint32_t>::iterator It =
- t.insert(std::make_pair("abcdef", 42)).first;
- EXPECT_EQ(2u, t.getNumBuckets());
- EXPECT_EQ("abcdef", It->first());
- EXPECT_EQ(42u, It->second);
-}
-
// Create a non-default constructable value
struct StringMapTestStruct {
StringMapTestStruct(int i) : i(i) {}
More information about the llvm-commits
mailing list