[ADT] Allow empty string in StringSet
authorSam Clegg <sbc@chromium.org>
Fri, 14 Feb 2020 04:25:55 +0000 (20:25 -0800)
committerSam Clegg <sbc@chromium.org>
Mon, 30 Mar 2020 19:59:34 +0000 (12:59 -0700)
Also add a test case to wasm-ld that asserts without this change.
Internally wasm-ld builds a StringMap of exported functions and it seems
like allowing empty string in the set is preferable to adding checks.

This assert looks like it was most likely just a historical accident.
It started life here purely to support InputLanguagesSet:

  eeac27e38c5c567d63bbfa5410620d955696491b

Then got extracted here:

  e57a4033385c5976cbb17af1e962b1224a61183b

Then got moved to AST here

  5c48bae209bcbd261886f63abac695b1e30544e6

With the `InLang` paramater name still intact which suggested is
InputLanguagesSet origins.

Differential Revision: https://reviews.llvm.org/D74589

llvm/include/llvm/ADT/StringSet.h
llvm/unittests/ADT/StringSetTest.cpp

index c740aee..7f3bbd4 100644 (file)
@@ -36,7 +36,6 @@ namespace llvm {
     explicit StringSet(AllocatorTy A) : base(A) {}
 
     std::pair<typename base::iterator, bool> insert(StringRef Key) {
-      assert(!Key.empty());
       return base::insert(std::make_pair(Key, None));
     }
 
index 17bfa1d..f3ec217 100644 (file)
@@ -41,4 +41,15 @@ TEST_F(StringSetTest, InsertAndCountStringMapEntry) {
   Element->Destroy();
 }
 
+TEST_F(StringSetTest, EmptyString) {
+  // Verify that the empty string can by successfully inserted
+  StringSet<> Set;
+  size_t Count = Set.count("");
+  EXPECT_EQ(Count, 0UL);
+
+  Set.insert("");
+  Count = Set.count("");
+  EXPECT_EQ(Count, 1UL);
+}
+
 } // end anonymous namespace