[PATCH] D98223: [lld-macho] implement options -(un)exported_symbol(s_list)

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:611
+          continue;
+      }
       trieBuilder.addSymbol(*defined);
----------------
This looks very branch and nested. How about:

```
static bool shouldExportSymbol(const Defined& defined) {
  if (!config->exportedSymbols.empty())
    return  config->exportedSymbols.match(defined->getName());

  return defined->privateExtern ||
            config->unexportedSymbols.match(defined->getName());
}

...

     if (!shouldExportSymbol(*defined))
       continue;
...
```

? This would also help with the issue below:


================
Comment at: lld/MachO/SyntheticSections.cpp:833
       uint8_t scope = 0;
       if (defined->privateExtern) {
         // Private external -- dylib scoped symbol.
----------------
You have to make a similar change here like you made in `finalizeContents()`, else the symbol table (what `nm -m`) won't match the export trie.

In general, tests that test `--exports-trie` output (or `--bind` output) should always also test `nm -m` output to make sure they're in sync.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98223/new/

https://reviews.llvm.org/D98223