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

[Keith Smiley via Phabricator via llvm-commits]

keith added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1415-1416
     error("cannot use both -exported_symbol* and -unexported_symbol* options\n"
           ">>> ignoring unexports");
     config->unexportedSymbols.clear();
   }
----------------
int3 wrote:
> tangential & existing code, but this seems kind of redundant to me lol. We're already erroring out, doesn't seem like there's a need to tell the user we're ignoring `unexports`. (Not sure there's a need to clear the unexported symbols list either, but I suppose it might make any subsequent error messages less confusing.)
Removed this, it was silently ignored anyways because of the order of the if statement below. If we find a case where error messages are degraded without it I guess we should add it back with a test showing that case.


================
Comment at: lld/MachO/Driver.cpp:1561
 
-    if (!config->exportedSymbols.empty()) {
+    if (!config->exportedSymbols.empty() || exportNoSymbols) {
       parallelForEach(symtab->getSymbols(), [](Symbol *sym) {
----------------
oontvoo wrote:
> 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?
> 
So I did the first suggestion here with the explicit exports, I'm not sure if it would be as clear if we joined exportedSymbols and unexportedSymbols but if folks generally prefer that I can change it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127562