[PATCH] D127562: [lld-macho] Add support for exporting no symbols

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1561
 
-    if (!config->exportedSymbols.empty()) {
+    if (!config->exportedSymbols.empty() || exportNoSymbols) {
       parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
----------------
int3 wrote:
> IMO this check is a bit confusing because it conflates a perf optimization with an actual difference in behavior. How about having the Config be something like
> 
> ```
> struct Config {
>   ...
>   bool explicitExports = false;
>   SymbolPatterns exportPatterns; // (can represent both `exportedSymbols` and `unexportedSymbols`)
> };
> ```
> 
> With `exportPatterns` interpreted differently based on the value of `explicitExports`. Then this `if` check becomes `if (config->explicitExports) { ...`
> 
> That said I know this is existing code, and refactoring it can be rightly said to be out of scope :)
agreed - perhaps get rid of the two unexportedSymbols and exportedSymbols lists and have just one list then a flag to say what the list means?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127562