[ADT] Allow empty string in StringSet

[sbc100]

Summary

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 history accident.
It started life here purely to support InputLanguagesSet:

eeac27e38c5c567d63bbfa5410620d955696491b

Then got extracted here:

e57a4033385c5976cbb17af1e962b1224a61183b

Then got moved to ADT here

5c48bae209bcbd261886f63abac695b1e30544e6

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

[dblaikie]

Probably best to test this functionality with a unit test for StringSet - but also please check if there's a reason StringSet might not support empty strings in its implementation in some way? (tombstone/other marker objects might be using "length 0" as a marker of some kind?)

[sbc100]

add test

[sbc100]

whitespace

[sbc100]

Added tests.

From inspection I can't see why this limitation would be needed. I also looked back though the history to identify the original reason for this limitation and it came, not from the data structure but from its domain-specific origin (a place where empty strings were not expected).

[dblaikie]

Sounds good - please commit the StringPool fix separately & I'd personally probably remove the wasm test, but I can see the merit in including it too, so up to you there.

I also looked back though the history to identify the original reason for this limitation and it came, not from the data structure but from its domain-specific origin (a place where empty strings were not expected).

Could you include a few of the details from the history there to explain it in the commit message?

[sbc100]

Will do

Could you include a few of the details from the history there to explain it in the commit message?

The change description goes into quite a bit of detail already no? Or am I missing something?

[sbc100]

Revert misc changes

[dblaikie]

In D74589#1947218, @sbc100 wrote:

Will do

Could you include a few of the details from the history there to explain it in the commit message?

The change description goes into quite a bit of detail already no? Or am I missing something?

ah, sure - that's quite sufficient - thanks!

lld/test/wasm/export-empty.test

+RUN: llc -filetype=obj %p/Inputs/start.ll -o %t.o
+RUN: not wasm-ld --export "" %t.o -o %t.wasm 2>&1 | FileCheck --match-full-lines %s
+
+CHECK: wasm-ld: error: symbol exported via --export not found: 

llvm/include/llvm/ADT/StringSet.h

public:
     StringSet() = default;
     StringSet(std::initializer_list<StringRef> S) {
       for (StringRef X : S)
         insert(X);
     }
     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));
     }
 
     template <typename InputIt>
     void insert(const InputIt &Begin, const InputIt &End) {
       for (auto It = Begin; It != End; ++It)
         base::insert(std::make_pair(*It, None));
     }