[llvm] r211309 - Add StringMap::insert(pair) consistent with the standard associative container concept.
Agustín K-ballo Bergé
kaballo86 at hotmail.com
Sun Jun 22 14:55:10 PDT 2014
On 20/06/2014 04:41 p.m., David Blaikie wrote:
> On Fri, Jun 20, 2014 at 11:41 AM, Agustín K-ballo Bergé
> <kaballo86 at hotmail.com> wrote:
>> On 19/06/2014 09:37 p.m., Rafael Espíndola wrote:
>>>
>>> Looks like this broke the build with gcc 4.7 in some bots and I reverted
>>> it.
>>>
>>> On 19 June 2014 16:08, David Blaikie <dblaikie at gmail.com> wrote:
>>>>
>>>> Author: dblaikie
>>>> Date: Thu Jun 19 15:08:56 2014
>>>> New Revision: 211309
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=211309&view=rev
>>>> Log:
>>>> Add StringMap::insert(pair) consistent with the standard associative
>>>> container concept.
>>>>
>>>> Patch by Agustín Bergé.
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/ADT/StringMap.h
>>>> llvm/trunk/lib/Support/StringMap.cpp
>>>> llvm/trunk/unittests/ADT/StringMapTest.cpp
>>>>
>>
>> It seems this is due to a regression in gcc 4.7 when movable-only types are
>> used in conjunction with `std::pair`. I was able to reproduce this with 4.7,
>> but not with 4.6 nor 4.8. Interestingly, this problem doesn't show if the
>> copy-constructor is defined private and deleted, it happens only when the
>> copy-constructor is declared private and not defined.
>>
>> Looks like the test case should have been used `LLVM_DELETED_FUNCTION`.
>
> I'm OK with using LLVM_DELETED_FUNCTION - it's better anyway from a
> diagnostic perspective and if it happens to workaround a buggy old
> compiler, I'm OK with that too.
>
>> Is
>> this an acceptable workaround (assuming it works), or is using movable-only
>> types in pairs just something to avoid because of this gcc 4.7 regression?
The attached patch adds `LLVM_DELETED_FUNCTION` to r211309. With those
changes, make runs successfully on gcc-4.7.
Regards,
--
Agustín K-ballo Bergé.-
http://talesofcpp.fusionfenix.com
-------------- next part --------------
Index: include/llvm/ADT/StringMap.h
===================================================================
--- include/llvm/ADT/StringMap.h (revision 211475)
+++ include/llvm/ADT/StringMap.h (working copy)
@@ -1,472 +1,477 @@
//===--- StringMap.h - String Hash table map interface ----------*- C++ -*-===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
//
// This file defines the StringMap class.
//
//===----------------------------------------------------------------------===//
#ifndef LLVM_ADT_STRINGMAP_H
#define LLVM_ADT_STRINGMAP_H
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Allocator.h"
#include <cstring>
#include <utility>
namespace llvm {
template<typename ValueT>
class StringMapConstIterator;
template<typename ValueT>
class StringMapIterator;
template<typename ValueTy>
class StringMapEntry;
/// StringMapEntryBase - Shared base class of StringMapEntry instances.
class StringMapEntryBase {
unsigned StrLen;
public:
explicit StringMapEntryBase(unsigned Len) : StrLen(Len) {}
unsigned getKeyLength() const { return StrLen; }
};
/// StringMapImpl - This is the base class of StringMap that is shared among
/// all of its instantiations.
class StringMapImpl {
protected:
// Array of NumBuckets pointers to entries, null pointers are holes.
// TheTable[NumBuckets] contains a sentinel value for easy iteration. Followed
// by an array of the actual hash values as unsigned integers.
StringMapEntryBase **TheTable;
unsigned NumBuckets;
unsigned NumItems;
unsigned NumTombstones;
unsigned ItemSize;
protected:
explicit StringMapImpl(unsigned itemSize)
: TheTable(nullptr),
// Initialize the map with zero buckets to allocation.
NumBuckets(0), NumItems(0), NumTombstones(0), ItemSize(itemSize) {}
StringMapImpl(StringMapImpl &&RHS)
: TheTable(RHS.TheTable), NumBuckets(RHS.NumBuckets),
NumItems(RHS.NumItems), NumTombstones(RHS.NumTombstones),
ItemSize(RHS.ItemSize) {
RHS.TheTable = nullptr;
RHS.NumBuckets = 0;
RHS.NumItems = 0;
RHS.NumTombstones = 0;
}
StringMapImpl(unsigned InitSize, unsigned ItemSize);
- void RehashTable();
+ unsigned RehashTable(unsigned BucketNo = 0);
/// 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
/// specified bucket will be non-null. Otherwise, it will be null. In either
/// case, the FullHashValue field of the bucket will be set to the hash value
/// of the string.
unsigned LookupBucketFor(StringRef Key);
/// FindKey - Look up the bucket that contains the specified key. If it exists
/// in the map, return the bucket number of the key. Otherwise return -1.
/// This does not modify the map.
int FindKey(StringRef Key) const;
/// RemoveKey - Remove the specified StringMapEntry from the table, but do not
/// delete it. This aborts if the value isn't in the table.
void RemoveKey(StringMapEntryBase *V);
/// RemoveKey - Remove the StringMapEntry for the specified key from the
/// table, returning it. If the key is not in the table, this returns null.
StringMapEntryBase *RemoveKey(StringRef Key);
private:
void init(unsigned Size);
public:
static StringMapEntryBase *getTombstoneVal() {
return (StringMapEntryBase*)-1;
}
unsigned getNumBuckets() const { return NumBuckets; }
unsigned getNumItems() const { return NumItems; }
bool empty() const { return NumItems == 0; }
unsigned size() const { return NumItems; }
void swap(StringMapImpl &Other) {
std::swap(TheTable, Other.TheTable);
std::swap(NumBuckets, Other.NumBuckets);
std::swap(NumItems, Other.NumItems);
std::swap(NumTombstones, Other.NumTombstones);
}
};
/// StringMapEntry - This is used to represent one value that is inserted into
/// a StringMap. It contains the Value itself and the key: the string length
/// and data.
template<typename ValueTy>
class StringMapEntry : public StringMapEntryBase {
StringMapEntry(StringMapEntry &E) LLVM_DELETED_FUNCTION;
public:
ValueTy second;
explicit StringMapEntry(unsigned strLen)
: StringMapEntryBase(strLen), second() {}
StringMapEntry(unsigned strLen, ValueTy V)
: StringMapEntryBase(strLen), second(std::move(V)) {}
StringRef getKey() const {
return StringRef(getKeyData(), getKeyLength());
}
const ValueTy &getValue() const { return second; }
ValueTy &getValue() { return second; }
void setValue(const ValueTy &V) { second = V; }
/// getKeyData - Return the start of the string data that is the key for this
/// value. The string data is always stored immediately after the
/// StringMapEntry object.
const char *getKeyData() const {return reinterpret_cast<const char*>(this+1);}
StringRef first() const { return StringRef(getKeyData(), getKeyLength()); }
/// Create - Create a StringMapEntry for the specified key and default
/// construct the value.
template<typename AllocatorTy, typename InitType>
static StringMapEntry *Create(StringRef Key,
AllocatorTy &Allocator,
InitType InitVal) {
unsigned KeyLength = Key.size();
// Allocate a new item with space for the string at the end and a null
// terminator.
unsigned AllocSize = static_cast<unsigned>(sizeof(StringMapEntry))+
KeyLength+1;
unsigned Alignment = alignOf<StringMapEntry>();
StringMapEntry *NewItem =
static_cast<StringMapEntry*>(Allocator.Allocate(AllocSize,Alignment));
// Default construct the value.
new (NewItem) StringMapEntry(KeyLength, std::move(InitVal));
// Copy the string information.
char *StrBuffer = const_cast<char*>(NewItem->getKeyData());
memcpy(StrBuffer, Key.data(), KeyLength);
StrBuffer[KeyLength] = 0; // Null terminate for convenience of clients.
return NewItem;
}
template<typename AllocatorTy>
static StringMapEntry *Create(StringRef Key, AllocatorTy &Allocator) {
return Create(Key, Allocator, ValueTy());
}
/// Create - Create a StringMapEntry with normal malloc/free.
template<typename InitType>
static StringMapEntry *Create(StringRef Key, InitType InitVal) {
MallocAllocator A;
return Create(Key, A, std::move(InitVal));
}
static StringMapEntry *Create(StringRef Key) {
return Create(Key, ValueTy());
}
/// GetStringMapEntryFromValue - Given a value that is known to be embedded
/// into a StringMapEntry, return the StringMapEntry itself.
static StringMapEntry &GetStringMapEntryFromValue(ValueTy &V) {
StringMapEntry *EPtr = 0;
char *Ptr = reinterpret_cast<char*>(&V) -
(reinterpret_cast<char*>(&EPtr->second) -
reinterpret_cast<char*>(EPtr));
return *reinterpret_cast<StringMapEntry*>(Ptr);
}
static const StringMapEntry &GetStringMapEntryFromValue(const ValueTy &V) {
return GetStringMapEntryFromValue(const_cast<ValueTy&>(V));
}
/// GetStringMapEntryFromKeyData - Given key data that is known to be embedded
/// into a StringMapEntry, return the StringMapEntry itself.
static StringMapEntry &GetStringMapEntryFromKeyData(const char *KeyData) {
char *Ptr = const_cast<char*>(KeyData) - sizeof(StringMapEntry<ValueTy>);
return *reinterpret_cast<StringMapEntry*>(Ptr);
}
/// Destroy - Destroy this StringMapEntry, releasing memory back to the
/// specified allocator.
template<typename AllocatorTy>
void Destroy(AllocatorTy &Allocator) {
// Free memory referenced by the item.
unsigned AllocSize =
static_cast<unsigned>(sizeof(StringMapEntry)) + getKeyLength() + 1;
this->~StringMapEntry();
Allocator.Deallocate(static_cast<void *>(this), AllocSize);
}
/// Destroy this object, releasing memory back to the malloc allocator.
void Destroy() {
MallocAllocator A;
Destroy(A);
}
};
/// StringMap - This is an unconventional map that is specialized for handling
/// keys that are "strings", which are basically ranges of bytes. This does some
/// funky memory allocation and hashing things to make it extremely efficient,
/// storing the string data *after* the value in the map.
template<typename ValueTy, typename AllocatorTy = MallocAllocator>
class StringMap : public StringMapImpl {
AllocatorTy Allocator;
public:
typedef StringMapEntry<ValueTy> MapEntryTy;
StringMap() : StringMapImpl(static_cast<unsigned>(sizeof(MapEntryTy))) {}
explicit StringMap(unsigned InitialSize)
: StringMapImpl(InitialSize, static_cast<unsigned>(sizeof(MapEntryTy))) {}
explicit StringMap(AllocatorTy A)
: StringMapImpl(static_cast<unsigned>(sizeof(MapEntryTy))), Allocator(A) {}
StringMap(unsigned InitialSize, AllocatorTy A)
: StringMapImpl(InitialSize, static_cast<unsigned>(sizeof(MapEntryTy))),
Allocator(A) {}
StringMap(StringMap &&RHS)
: StringMapImpl(std::move(RHS)), Allocator(std::move(RHS.Allocator)) {}
StringMap &operator=(StringMap RHS) {
StringMapImpl::swap(RHS);
std::swap(Allocator, RHS.Allocator);
return *this;
}
// FIXME: Implement copy operations if/when they're needed.
AllocatorTy &getAllocator() { return Allocator; }
const AllocatorTy &getAllocator() const { return Allocator; }
typedef const char* key_type;
typedef ValueTy mapped_type;
typedef StringMapEntry<ValueTy> value_type;
typedef size_t size_type;
typedef StringMapConstIterator<ValueTy> const_iterator;
typedef StringMapIterator<ValueTy> iterator;
iterator begin() {
return iterator(TheTable, NumBuckets == 0);
}
iterator end() {
return iterator(TheTable+NumBuckets, true);
}
const_iterator begin() const {
return const_iterator(TheTable, NumBuckets == 0);
}
const_iterator end() const {
return const_iterator(TheTable+NumBuckets, true);
}
iterator find(StringRef Key) {
int Bucket = FindKey(Key);
if (Bucket == -1) return end();
return iterator(TheTable+Bucket, true);
}
const_iterator find(StringRef Key) const {
int Bucket = FindKey(Key);
if (Bucket == -1) return end();
return const_iterator(TheTable+Bucket, true);
}
/// lookup - Return the entry for the specified key, or a default
/// constructed value if no such entry exists.
ValueTy lookup(StringRef Key) const {
const_iterator it = find(Key);
if (it != end())
return it->second;
return ValueTy();
}
ValueTy &operator[](StringRef Key) {
return GetOrCreateValue(Key).getValue();
}
/// count - Return 1 if the element is in the map, 0 otherwise.
size_type count(StringRef Key) const {
return find(Key) == end() ? 0 : 1;
}
/// insert - Insert the specified key/value pair into the map. If the key
/// already exists in the map, return false and ignore the request, otherwise
/// insert it and return true.
bool insert(MapEntryTy *KeyValue) {
unsigned BucketNo = LookupBucketFor(KeyValue->getKey());
StringMapEntryBase *&Bucket = TheTable[BucketNo];
if (Bucket && Bucket != getTombstoneVal())
return false; // Already exists in map.
if (Bucket == getTombstoneVal())
--NumTombstones;
Bucket = KeyValue;
++NumItems;
assert(NumItems + NumTombstones <= NumBuckets);
RehashTable();
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;
// Zap all values, resetting the keys back to non-present (not tombstone),
// which is safe because we're removing all elements.
for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
StringMapEntryBase *&Bucket = TheTable[I];
if (Bucket && Bucket != getTombstoneVal()) {
static_cast<MapEntryTy*>(Bucket)->Destroy(Allocator);
}
Bucket = nullptr;
}
NumItems = 0;
NumTombstones = 0;
}
/// GetOrCreateValue - Look up the specified key in the table. If a value
/// exists, return it. Otherwise, default construct a value, insert it, and
/// return.
template <typename InitTy>
MapEntryTy &GetOrCreateValue(StringRef Key, InitTy Val) {
- 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;
+ return *insert(std::make_pair(Key, std::move(Val))).first;
}
MapEntryTy &GetOrCreateValue(StringRef Key) {
return GetOrCreateValue(Key, ValueTy());
}
/// remove - Remove the specified key/value pair from the map, but do not
/// erase it. This aborts if the key is not in the map.
void remove(MapEntryTy *KeyValue) {
RemoveKey(KeyValue);
}
void erase(iterator I) {
MapEntryTy &V = *I;
remove(&V);
V.Destroy(Allocator);
}
bool erase(StringRef Key) {
iterator I = find(Key);
if (I == end()) return false;
erase(I);
return true;
}
~StringMap() {
// Delete all the elements in the map, but don't reset the elements
// to default values. This is a copy of clear(), but avoids unnecessary
// work not required in the destructor.
if (!empty()) {
for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
StringMapEntryBase *Bucket = TheTable[I];
if (Bucket && Bucket != getTombstoneVal()) {
static_cast<MapEntryTy*>(Bucket)->Destroy(Allocator);
}
}
}
free(TheTable);
}
};
template<typename ValueTy>
class StringMapConstIterator {
protected:
StringMapEntryBase **Ptr;
public:
typedef StringMapEntry<ValueTy> value_type;
StringMapConstIterator() : Ptr(nullptr) { }
explicit StringMapConstIterator(StringMapEntryBase **Bucket,
bool NoAdvance = false)
: Ptr(Bucket) {
if (!NoAdvance) AdvancePastEmptyBuckets();
}
const value_type &operator*() const {
return *static_cast<StringMapEntry<ValueTy>*>(*Ptr);
}
const value_type *operator->() const {
return static_cast<StringMapEntry<ValueTy>*>(*Ptr);
}
bool operator==(const StringMapConstIterator &RHS) const {
return Ptr == RHS.Ptr;
}
bool operator!=(const StringMapConstIterator &RHS) const {
return Ptr != RHS.Ptr;
}
inline StringMapConstIterator& operator++() { // Preincrement
++Ptr;
AdvancePastEmptyBuckets();
return *this;
}
StringMapConstIterator operator++(int) { // Postincrement
StringMapConstIterator tmp = *this; ++*this; return tmp;
}
private:
void AdvancePastEmptyBuckets() {
while (*Ptr == nullptr || *Ptr == StringMapImpl::getTombstoneVal())
++Ptr;
}
};
template<typename ValueTy>
class StringMapIterator : public StringMapConstIterator<ValueTy> {
public:
StringMapIterator() {}
explicit StringMapIterator(StringMapEntryBase **Bucket,
bool NoAdvance = false)
: StringMapConstIterator<ValueTy>(Bucket, NoAdvance) {
}
StringMapEntry<ValueTy> &operator*() const {
return *static_cast<StringMapEntry<ValueTy>*>(*this->Ptr);
}
StringMapEntry<ValueTy> *operator->() const {
return static_cast<StringMapEntry<ValueTy>*>(*this->Ptr);
}
};
}
#endif
Index: lib/Support/StringMap.cpp
===================================================================
--- lib/Support/StringMap.cpp (revision 211475)
+++ lib/Support/StringMap.cpp (working copy)
@@ -1,238 +1,244 @@
//===--- StringMap.cpp - String Hash table map implementation -------------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
//
// This file implements the StringMap class.
//
//===----------------------------------------------------------------------===//
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Compiler.h"
#include <cassert>
using namespace llvm;
StringMapImpl::StringMapImpl(unsigned InitSize, unsigned itemSize) {
ItemSize = itemSize;
// If a size is specified, initialize the table with that many buckets.
if (InitSize) {
init(InitSize);
return;
}
// Otherwise, initialize it with zero buckets to avoid the allocation.
TheTable = nullptr;
NumBuckets = 0;
NumItems = 0;
NumTombstones = 0;
}
void StringMapImpl::init(unsigned InitSize) {
assert((InitSize & (InitSize-1)) == 0 &&
"Init Size must be a power of 2 or zero!");
NumBuckets = InitSize ? InitSize : 16;
NumItems = 0;
NumTombstones = 0;
TheTable = (StringMapEntryBase **)calloc(NumBuckets+1,
sizeof(StringMapEntryBase **) +
sizeof(unsigned));
// Allocate one extra bucket, set it to look filled so the iterators stop at
// end.
TheTable[NumBuckets] = (StringMapEntryBase*)2;
}
/// 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
/// specified bucket will be non-null. Otherwise, it will be null. In either
/// case, the FullHashValue field of the bucket will be set to the hash value
/// of the string.
unsigned StringMapImpl::LookupBucketFor(StringRef Name) {
unsigned HTSize = NumBuckets;
if (HTSize == 0) { // Hash table unallocated so far?
init(16);
HTSize = NumBuckets;
}
unsigned FullHashValue = HashString(Name);
unsigned BucketNo = FullHashValue & (HTSize-1);
unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
unsigned ProbeAmt = 1;
int FirstTombstone = -1;
while (1) {
StringMapEntryBase *BucketItem = TheTable[BucketNo];
// If we found an empty bucket, this key isn't in the table yet, return it.
if (LLVM_LIKELY(!BucketItem)) {
// If we found a tombstone, we want to reuse the tombstone instead of an
// empty bucket. This reduces probing.
if (FirstTombstone != -1) {
HashTable[FirstTombstone] = FullHashValue;
return FirstTombstone;
}
HashTable[BucketNo] = FullHashValue;
return BucketNo;
}
if (BucketItem == getTombstoneVal()) {
// Skip over tombstones. However, remember the first one we see.
if (FirstTombstone == -1) FirstTombstone = BucketNo;
} else if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
// If the full hash value matches, check deeply for a match. The common
// case here is that we are only looking at the buckets (for item info
// being non-null and for the full hash value) not at the items. This
// is important for cache locality.
// Do the comparison like this because Name isn't necessarily
// null-terminated!
char *ItemStr = (char*)BucketItem+ItemSize;
if (Name == StringRef(ItemStr, BucketItem->getKeyLength())) {
// We found a match!
return BucketNo;
}
}
// Okay, we didn't find the item. Probe to the next bucket.
BucketNo = (BucketNo+ProbeAmt) & (HTSize-1);
// Use quadratic probing, it has fewer clumping artifacts than linear
// probing and has good cache behavior in the common case.
++ProbeAmt;
}
}
/// FindKey - Look up the bucket that contains the specified key. If it exists
/// in the map, return the bucket number of the key. Otherwise return -1.
/// This does not modify the map.
int StringMapImpl::FindKey(StringRef Key) const {
unsigned HTSize = NumBuckets;
if (HTSize == 0) return -1; // Really empty table?
unsigned FullHashValue = HashString(Key);
unsigned BucketNo = FullHashValue & (HTSize-1);
unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
unsigned ProbeAmt = 1;
while (1) {
StringMapEntryBase *BucketItem = TheTable[BucketNo];
// If we found an empty bucket, this key isn't in the table yet, return.
if (LLVM_LIKELY(!BucketItem))
return -1;
if (BucketItem == getTombstoneVal()) {
// Ignore tombstones.
} else if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
// If the full hash value matches, check deeply for a match. The common
// case here is that we are only looking at the buckets (for item info
// being non-null and for the full hash value) not at the items. This
// is important for cache locality.
// Do the comparison like this because NameStart isn't necessarily
// null-terminated!
char *ItemStr = (char*)BucketItem+ItemSize;
if (Key == StringRef(ItemStr, BucketItem->getKeyLength())) {
// We found a match!
return BucketNo;
}
}
// Okay, we didn't find the item. Probe to the next bucket.
BucketNo = (BucketNo+ProbeAmt) & (HTSize-1);
// Use quadratic probing, it has fewer clumping artifacts than linear
// probing and has good cache behavior in the common case.
++ProbeAmt;
}
}
/// RemoveKey - Remove the specified StringMapEntry from the table, but do not
/// delete it. This aborts if the value isn't in the table.
void StringMapImpl::RemoveKey(StringMapEntryBase *V) {
const char *VStr = (char*)V + ItemSize;
StringMapEntryBase *V2 = RemoveKey(StringRef(VStr, V->getKeyLength()));
(void)V2;
assert(V == V2 && "Didn't find key?");
}
/// RemoveKey - Remove the StringMapEntry for the specified key from the
/// table, returning it. If the key is not in the table, this returns null.
StringMapEntryBase *StringMapImpl::RemoveKey(StringRef Key) {
int Bucket = FindKey(Key);
if (Bucket == -1) return nullptr;
StringMapEntryBase *Result = TheTable[Bucket];
TheTable[Bucket] = getTombstoneVal();
--NumItems;
++NumTombstones;
assert(NumItems + NumTombstones <= NumBuckets);
return Result;
}
/// RehashTable - Grow the table, redistributing values into the buckets with
/// the appropriate mod-of-hashtable-size.
-void StringMapImpl::RehashTable() {
+unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
unsigned NewSize;
unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1);
// If the hash table is now more than 3/4 full, or if fewer than 1/8 of
// the buckets are empty (meaning that many are filled with tombstones),
// grow/rehash the table.
if (NumItems*4 > NumBuckets*3) {
NewSize = NumBuckets*2;
} else if (NumBuckets-(NumItems+NumTombstones) <= NumBuckets/8) {
NewSize = NumBuckets;
} else {
- return;
+ return BucketNo;
}
+ unsigned NewBucketNo = BucketNo;
// Allocate one extra bucket which will always be non-empty. This allows the
// iterators to stop at end.
StringMapEntryBase **NewTableArray =
(StringMapEntryBase **)calloc(NewSize+1, sizeof(StringMapEntryBase *) +
sizeof(unsigned));
unsigned *NewHashArray = (unsigned *)(NewTableArray + NewSize + 1);
NewTableArray[NewSize] = (StringMapEntryBase*)2;
// Rehash all the items into their new buckets. Luckily :) we already have
// the hash values available, so we don't have to rehash any strings.
for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
StringMapEntryBase *Bucket = TheTable[I];
if (Bucket && Bucket != getTombstoneVal()) {
// Fast case, bucket available.
unsigned FullHash = HashTable[I];
unsigned NewBucket = FullHash & (NewSize-1);
if (!NewTableArray[NewBucket]) {
NewTableArray[FullHash & (NewSize-1)] = Bucket;
NewHashArray[FullHash & (NewSize-1)] = FullHash;
+ if (I == BucketNo)
+ NewBucketNo = NewBucket;
continue;
}
// Otherwise probe for a spot.
unsigned ProbeSize = 1;
do {
NewBucket = (NewBucket + ProbeSize++) & (NewSize-1);
} while (NewTableArray[NewBucket]);
// Finally found a slot. Fill it in.
NewTableArray[NewBucket] = Bucket;
NewHashArray[NewBucket] = FullHash;
+ if (I == BucketNo)
+ NewBucketNo = NewBucket;
}
}
free(TheTable);
TheTable = NewTableArray;
NumBuckets = NewSize;
NumTombstones = 0;
+ return NewBucketNo;
}
Index: unittests/ADT/StringMapTest.cpp
===================================================================
--- unittests/ADT/StringMapTest.cpp (revision 211475)
+++ unittests/ADT/StringMapTest.cpp (working copy)
@@ -1,309 +1,347 @@
//===- llvm/unittest/ADT/StringMapMap.cpp - StringMap unit tests ----------===//
//
// The LLVM Compiler Infrastructure
//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
//===----------------------------------------------------------------------===//
#include "gtest/gtest.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/DataTypes.h"
+#include <tuple>
using namespace llvm;
namespace {
// Test fixture
class StringMapTest : public testing::Test {
protected:
StringMap<uint32_t> testMap;
static const char testKey[];
static const uint32_t testValue;
static const char* testKeyFirst;
static size_t testKeyLength;
static const std::string testKeyStr;
void assertEmptyMap() {
// Size tests
EXPECT_EQ(0u, testMap.size());
EXPECT_TRUE(testMap.empty());
// Iterator tests
EXPECT_TRUE(testMap.begin() == testMap.end());
// Lookup tests
EXPECT_EQ(0u, testMap.count(testKey));
EXPECT_EQ(0u, testMap.count(StringRef(testKeyFirst, testKeyLength)));
EXPECT_EQ(0u, testMap.count(testKeyStr));
EXPECT_TRUE(testMap.find(testKey) == testMap.end());
EXPECT_TRUE(testMap.find(StringRef(testKeyFirst, testKeyLength)) ==
testMap.end());
EXPECT_TRUE(testMap.find(testKeyStr) == testMap.end());
}
void assertSingleItemMap() {
// Size tests
EXPECT_EQ(1u, testMap.size());
EXPECT_FALSE(testMap.begin() == testMap.end());
EXPECT_FALSE(testMap.empty());
// Iterator tests
StringMap<uint32_t>::iterator it = testMap.begin();
EXPECT_STREQ(testKey, it->first().data());
EXPECT_EQ(testValue, it->second);
++it;
EXPECT_TRUE(it == testMap.end());
// Lookup tests
EXPECT_EQ(1u, testMap.count(testKey));
EXPECT_EQ(1u, testMap.count(StringRef(testKeyFirst, testKeyLength)));
EXPECT_EQ(1u, testMap.count(testKeyStr));
EXPECT_TRUE(testMap.find(testKey) == testMap.begin());
EXPECT_TRUE(testMap.find(StringRef(testKeyFirst, testKeyLength)) ==
testMap.begin());
EXPECT_TRUE(testMap.find(testKeyStr) == testMap.begin());
}
};
const char StringMapTest::testKey[] = "key";
const uint32_t StringMapTest::testValue = 1u;
const char* StringMapTest::testKeyFirst = testKey;
size_t StringMapTest::testKeyLength = sizeof(testKey) - 1;
const std::string StringMapTest::testKeyStr(testKey);
// Empty map tests.
TEST_F(StringMapTest, EmptyMapTest) {
assertEmptyMap();
}
// Constant map tests.
TEST_F(StringMapTest, ConstEmptyMapTest) {
const StringMap<uint32_t>& constTestMap = testMap;
// Size tests
EXPECT_EQ(0u, constTestMap.size());
EXPECT_TRUE(constTestMap.empty());
// Iterator tests
EXPECT_TRUE(constTestMap.begin() == constTestMap.end());
// Lookup tests
EXPECT_EQ(0u, constTestMap.count(testKey));
EXPECT_EQ(0u, constTestMap.count(StringRef(testKeyFirst, testKeyLength)));
EXPECT_EQ(0u, constTestMap.count(testKeyStr));
EXPECT_TRUE(constTestMap.find(testKey) == constTestMap.end());
EXPECT_TRUE(constTestMap.find(StringRef(testKeyFirst, testKeyLength)) ==
constTestMap.end());
EXPECT_TRUE(constTestMap.find(testKeyStr) == constTestMap.end());
}
// A map with a single entry.
TEST_F(StringMapTest, SingleEntryMapTest) {
testMap[testKey] = testValue;
assertSingleItemMap();
}
// Test clear() method.
TEST_F(StringMapTest, ClearTest) {
testMap[testKey] = testValue;
testMap.clear();
assertEmptyMap();
}
// Test erase(iterator) method.
TEST_F(StringMapTest, EraseIteratorTest) {
testMap[testKey] = testValue;
testMap.erase(testMap.begin());
assertEmptyMap();
}
// Test erase(value) method.
TEST_F(StringMapTest, EraseValueTest) {
testMap[testKey] = testValue;
testMap.erase(testKey);
assertEmptyMap();
}
// Test inserting two values and erasing one.
TEST_F(StringMapTest, InsertAndEraseTest) {
testMap[testKey] = testValue;
testMap["otherKey"] = 2;
testMap.erase("otherKey");
assertSingleItemMap();
}
TEST_F(StringMapTest, SmallFullMapTest) {
// StringMap has a tricky corner case when the map is small (<8 buckets) and
// it fills up through a balanced pattern of inserts and erases. This can
// lead to inf-loops in some cases (PR13148) so we test it explicitly here.
llvm::StringMap<int> Map(2);
Map["eins"] = 1;
Map["zwei"] = 2;
Map["drei"] = 3;
Map.erase("drei");
Map.erase("eins");
Map["veir"] = 4;
Map["funf"] = 5;
EXPECT_EQ(3u, Map.size());
EXPECT_EQ(0, Map.lookup("eins"));
EXPECT_EQ(2, Map.lookup("zwei"));
EXPECT_EQ(0, Map.lookup("drei"));
EXPECT_EQ(4, Map.lookup("veir"));
EXPECT_EQ(5, Map.lookup("funf"));
}
// A more complex iteration test.
TEST_F(StringMapTest, IterationTest) {
bool visited[100];
// Insert 100 numbers into the map
for (int i = 0; i < 100; ++i) {
std::stringstream ss;
ss << "key_" << i;
testMap[ss.str()] = i;
visited[i] = false;
}
// Iterate over all numbers and mark each one found.
for (StringMap<uint32_t>::iterator it = testMap.begin();
it != testMap.end(); ++it) {
std::stringstream ss;
ss << "key_" << it->second;
ASSERT_STREQ(ss.str().c_str(), it->first().data());
visited[it->second] = true;
}
// Ensure every number was visited.
for (int i = 0; i < 100; ++i) {
ASSERT_TRUE(visited[i]) << "Entry #" << i << " was never visited";
}
}
// Test StringMapEntry::Create() method.
TEST_F(StringMapTest, StringMapEntryTest) {
StringMap<uint32_t>::value_type* entry =
StringMap<uint32_t>::value_type::Create(
StringRef(testKeyFirst, testKeyLength), 1u);
EXPECT_STREQ(testKey, entry->first().data());
EXPECT_EQ(1u, entry->second);
free(entry);
}
// Test insert() method.
TEST_F(StringMapTest, InsertTest) {
SCOPED_TRACE("InsertTest");
testMap.insert(
StringMap<uint32_t>::value_type::Create(
StringRef(testKeyFirst, testKeyLength),
testMap.getAllocator(), 1u));
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) {}
StringMapTestStruct() LLVM_DELETED_FUNCTION;
int i;
};
TEST_F(StringMapTest, NonDefaultConstructable) {
StringMap<StringMapTestStruct> t;
t.GetOrCreateValue("Test", StringMapTestStruct(123));
StringMap<StringMapTestStruct>::iterator iter = t.find("Test");
ASSERT_NE(iter, t.end());
ASSERT_EQ(iter->second.i, 123);
}
struct MoveOnly {
int i;
MoveOnly(int i) : i(i) {}
MoveOnly(MoveOnly &&RHS) : i(RHS.i) {}
MoveOnly &operator=(MoveOnly &&RHS) {
i = RHS.i;
return *this;
}
private:
- MoveOnly(const MoveOnly &);
- MoveOnly &operator=(const MoveOnly &);
+ MoveOnly(const MoveOnly &) LLVM_DELETED_FUNCTION;
+ MoveOnly &operator=(const MoveOnly &) LLVM_DELETED_FUNCTION;
};
TEST_F(StringMapTest, MoveOnlyKey) {
StringMap<MoveOnly> t;
t.GetOrCreateValue("Test", MoveOnly(42));
StringRef Key = "Test";
StringMapEntry<MoveOnly>::Create(Key, MoveOnly(42))
->Destroy();
}
TEST_F(StringMapTest, MoveConstruct) {
StringMap<int> A;
A.GetOrCreateValue("x", 42);
StringMap<int> B = std::move(A);
ASSERT_EQ(A.size(), 0u);
ASSERT_EQ(B.size(), 1u);
ASSERT_EQ(B["x"], 42);
ASSERT_EQ(B.count("y"), 0u);
}
TEST_F(StringMapTest, MoveAssignment) {
StringMap<int> A;
A["x"] = 42;
StringMap<int> B;
B["y"] = 117;
A = std::move(B);
ASSERT_EQ(A.size(), 1u);
ASSERT_EQ(B.size(), 0u);
ASSERT_EQ(A["y"], 117);
ASSERT_EQ(B.count("x"), 0u);
}
struct Countable {
int &InstanceCount;
int Number;
Countable(int Number, int &InstanceCount)
: InstanceCount(InstanceCount), Number(Number) {
++InstanceCount;
}
Countable(Countable &&C) : InstanceCount(C.InstanceCount), Number(C.Number) {
++InstanceCount;
C.Number = -1;
}
Countable(const Countable &C)
: InstanceCount(C.InstanceCount), Number(C.Number) {
++InstanceCount;
}
Countable &operator=(Countable C) {
Number = C.Number;
return *this;
}
~Countable() { --InstanceCount; }
};
TEST_F(StringMapTest, MoveDtor) {
int InstanceCount = 0;
StringMap<Countable> A;
A.GetOrCreateValue("x", Countable(42, InstanceCount));
ASSERT_EQ(InstanceCount, 1);
auto I = A.find("x");
ASSERT_NE(I, A.end());
ASSERT_EQ(I->second.Number, 42);
StringMap<Countable> B;
B = std::move(A);
ASSERT_EQ(InstanceCount, 1);
ASSERT_TRUE(A.empty());
I = B.find("x");
ASSERT_NE(I, B.end());
ASSERT_EQ(I->second.Number, 42);
B = StringMap<Countable>();
ASSERT_EQ(InstanceCount, 0);
ASSERT_TRUE(B.empty());
}
} // end anonymous namespace
More information about the llvm-commits
mailing list