[llvm] de77d23 - [ADT] Allow empty string in StringSet

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 30 12:59:47 PDT 2020


Author: Sam Clegg
Date: 2020-03-30T12:59:34-07:00
New Revision: de77d2312751514ce9fbf9b0bf11cbe93cd486f9

URL: https://github.com/llvm/llvm-project/commit/de77d2312751514ce9fbf9b0bf11cbe93cd486f9
DIFF: https://github.com/llvm/llvm-project/commit/de77d2312751514ce9fbf9b0bf11cbe93cd486f9.diff

LOG: [ADT] Allow empty string in StringSet

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/StringSet.h b/llvm/include/llvm/ADT/StringSet.h
index c740aee41a31..7f3bbd490171 100644
--- a/llvm/include/llvm/ADT/StringSet.h
+++ b/llvm/include/llvm/ADT/StringSet.h
@@ -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));
     }
 

diff  --git a/llvm/unittests/ADT/StringSetTest.cpp b/llvm/unittests/ADT/StringSetTest.cpp
index 17bfa1dd18d2..f3ec217779ec 100644
--- a/llvm/unittests/ADT/StringSetTest.cpp
+++ b/llvm/unittests/ADT/StringSetTest.cpp
@@ -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


        


More information about the llvm-commits mailing list