[PATCH] D18506: Add a copy constructor to StringMap

[Mehdi AMINI via llvm-commits]

joker.eph added inline comments.

================
Comment at: include/llvm/ADT/StringMap.h:248
@@ -246,2 +247,3 @@
 
-  // FIXME: Implement copy operations if/when they're needed.
+  StringMap(const StringMap &RHS) :
+    StringMapImpl(static_cast<unsigned>(sizeof(MapEntryTy))),
----------------
May be explicit?

================
Comment at: include/llvm/ADT/StringMap.h:261
@@ +260,3 @@
+    NumItems = RHS.NumItems;
+    NumTombstones = RHS.NumTombstones;
+    for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
----------------
I wonder if we really want to keep the tombstones or if it wouldn't be better to reinsert the elements?

================
Comment at: include/llvm/ADT/StringMap.h:272
@@ +271,3 @@
+      HashTable[I] = RHSHashTable[I];
+    }
+  }
----------------
The hashes have to be inserted as well I think (after the buckets), I may miss where it happened?


http://reviews.llvm.org/D18506