[PATCH] D126046: [lld-macho] Support -non_global_symbols_strip_list, -non_global_symbols_no_strip_list, -x

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:971
+    break;
+  case SymtabPresence::None:
+    localSymbolsHandler = [&](Symbol *) { /* Do nothing*/ };
----------------
thevinster wrote:
> Does this case need to be enumerated if it's already being filtered out below? 
>  Does this case need to be enumerated if it's already being filtered out below?

Yes, because we don't need to do the check on line 1009 and line 1019. We only do that check on line 991+ is because iterating through all the inputfiles and their symbols is expensive - hence if we can avoid it (ie. when -x is set), then we should




================
Comment at: lld/test/MachO/local-symbol-output.s:73
+
+## Test interactions with exported_symbol.
+# RUN: %lld %t/main.o -o %t/strip_all_export_one.out \
----------------
thevinster wrote:
> Can we also have a test for interactions with `-unexported_symbol` as well? Specifically, that a global doesn't get included in the symbol table?
I'm not sure I understand the test you're proposing. Can you give an example?
 Specifically, -x and -non_global_symbols_[no_]strip_list options don't have an effect on global symbols. So how would using them with -unexported_symbol be meaningful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126046