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

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/MachO/Config.h:53-54
+class SymbolPatterns {
+  llvm::DenseSet<llvm::StringRef> literals;
+  std::vector<llvm::GlobPattern> globs;
+
----------------
Hmm, GlobPattern.cpp has a special case for patterns without wildcards, but I guess having a separate literals set is still faster with the O(1) lookup

Since we're optimizing for speed here, I would use `CachedHashStringRef` for the literals set


================
Comment at: lld/MachO/Driver.cpp:741
+bool SymbolPatterns::matchGlob(StringRef symbolName) const {
+  for (auto const &glob : globs)
+    if (glob.match(symbolName))
----------------
avoid `auto`


================
Comment at: lld/test/MachO/export-options.s:11-12
+
+CONFLICT: error: cannot use both -exported_symbol* and -unexported_symbol* options
+CONFLICT-NEXT: >>> ignoring unexports
+
----------------
the lack of comment character works because split-file means it isn't passed into llvm-mc, but we should still make it a comment for uniformity


================
Comment at: lld/test/MachO/export-options.s:16-20
+.macro DEFSYM, type, sym
+\type \sym
+\sym:
+  retq
+.endm
----------------
I'm not sure I see much value in these macros... you're not really saving that many characters in exchange for an additional level of indirection

Without macros, you could save on the `retq`s by doing

```
_keep_globl:
_hide_globl:
_keep_private:
_show_private:
  retq
```

just my 2c, keep the macros if you like them


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98223