[PATCH] D38760: [LLD] [COFF] Add support for automatically exporting all symbols

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

Isn't there a hard limit on the number of files that can be exported in COFF?



================
Comment at: COFF/Driver.cpp:1214
+                      Args.hasArg(OPT_export_all_symbols))) {
+    std::vector<std::string> ExcludeSymbols = {
+        "_NULL_IMPORT_DESCRIPTOR",
----------------
ruiu wrote:
> Can you make this a separate function which returns a std::set?
Can you make this a `vector<StringRef>`, but add an additional `_` to the beginning of each item?

Then, below you can write:

```
if (Config->Machine != I386) {
  for (auto &Str : ExcludeSymbols) {
    Str = Str.drop_front().substr(0, Str.find('@'));
  }
}
```

saving unnecessary allocations on all code paths.


================
Comment at: COFF/Driver.cpp:1243-1244
+    }
+    std::set<StringRef> ExcludeSymbolSet(ExcludeSymbols.begin(),
+                                         ExcludeSymbols.end());
+
----------------
Since you've explicitly defined this set above, and there is no way to get additional items into it, why do we need a set?  Seems like you can just `std::sort` the original vector and then `std::binary_search` for symbols.


https://reviews.llvm.org/D38760