From 38bf13d02ce7dda0a9fc77c94457da0be8cf5c1b Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Wed, 30 Mar 2016 19:54:56 +0000 Subject: [PATCH] Add a copy constructor to StringMap There is code under review that requires StringMap to have a copy constructor, and this makes StringMap more consistent with our other containers (like DenseMap) that have copy constructors. Differential Revision: http://reviews.llvm.org/D18506 llvm-svn: 264906 --- llvm/include/llvm/ADT/StringMap.h | 37 ++++++++++++++++++++++++++++++++++-- llvm/unittests/ADT/StringMapTest.cpp | 27 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h index 3a3ffc2..ce80484 100644 --- a/llvm/include/llvm/ADT/StringMap.h +++ b/llvm/include/llvm/ADT/StringMap.h @@ -89,7 +89,8 @@ protected: /// table, returning it. If the key is not in the table, this returns null. StringMapEntryBase *RemoveKey(StringRef Key); -private: + /// Allocate the table with the specified number of buckets and otherwise + /// setup the map as empty. void init(unsigned Size); public: @@ -244,7 +245,39 @@ public: return *this; } - // FIXME: Implement copy operations if/when they're needed. + StringMap(const StringMap &RHS) : + StringMapImpl(static_cast(sizeof(MapEntryTy))), + Allocator(RHS.Allocator) { + if (RHS.empty()) + return; + + // Allocate TheTable of the same size as RHS's TheTable, and set the + // sentinel appropriately (and NumBuckets). + init(RHS.NumBuckets); + unsigned *HashTable = (unsigned *)(TheTable + NumBuckets + 1), + *RHSHashTable = (unsigned *)(RHS.TheTable + NumBuckets + 1); + + NumItems = RHS.NumItems; + NumTombstones = RHS.NumTombstones; + for (unsigned I = 0, E = NumBuckets; I != E; ++I) { + MapEntryTy *Bucket = ((MapEntryTy**) RHS.TheTable)[I]; + if (!Bucket || Bucket == getTombstoneVal()) { + TheTable[I] = Bucket; + continue; + } + + TheTable[I] = MapEntryTy::Create(Bucket->getKey(), Allocator, + Bucket->getValue()); + HashTable[I] = RHSHashTable[I]; + } + + // Note that here we've copied everything from the RHS into this object, + // tombstones included. We could, instead, have re-probed for each key to + // instantiate this new object without any tombstone buckets. The + // assumption here is that items are rarely deleted from most StringMaps, + // and so tombstones are rare, so the cost of re-probing for all inputs is + // not worthwhile. + } AllocatorTy &getAllocator() { return Allocator; } const AllocatorTy &getAllocator() const { return Allocator; } diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp index 562126e..a85cae1 100644 --- a/llvm/unittests/ADT/StringMapTest.cpp +++ b/llvm/unittests/ADT/StringMapTest.cpp @@ -157,6 +157,33 @@ TEST_F(StringMapTest, SmallFullMapTest) { EXPECT_EQ(5, Map.lookup("funf")); } +TEST_F(StringMapTest, CopyCtorTest) { + llvm::StringMap Map; + + 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")); + + llvm::StringMap Map2(Map); + EXPECT_EQ(3u, Map2.size()); + EXPECT_EQ(0, Map2.lookup("eins")); + EXPECT_EQ(2, Map2.lookup("zwei")); + EXPECT_EQ(0, Map2.lookup("drei")); + EXPECT_EQ(4, Map2.lookup("veir")); + EXPECT_EQ(5, Map2.lookup("funf")); +} + // A more complex iteration test. TEST_F(StringMapTest, IterationTest) { bool visited[100]; -- 2.7.4