[PATCH] D103430: [lld/mac] Implement removal of unused dylibs

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.h:162
+  bool isReferenced() const {
+    assert(numReferencedSymbols >= 0);
+    return numReferencedSymbols > 0;
----------------
why not just make it unsigned, and check that we don't call `unreference()` if it's already zero?


================
Comment at: lld/MachO/Symbols.h:251
+    assert(newState > RefState::Unreferenced);
+    if (refState == RefState::Unreferenced && file)
+      getFile()->numReferencedSymbols++;
----------------
can we have a comment about why the `&& file` check is necessary?


================
Comment at: lld/MachO/Symbols.h:255
+  }
+  void unreference() {
+    if (refState > RefState::Unreferenced && file)
----------------
ultra nit: can we have a newline between multi-line functions?


================
Comment at: lld/test/MachO/dead-strip-dylibs.s:48
+
+## LC_LINKER_OPTION does not count as explicit reference.
+# RUN: llvm-mc %t/linkopt_bar.s -triple=x86_64-apple-macos -filetype=obj -o %t/linkopt_bar.o
----------------



================
Comment at: lld/test/MachO/implicit-dylibs.s:77
 # LOAD-NEXT:    name /usr/lib/libc++abi.dylib
-# LOAD:          cmd LC_LOAD_DYLIB
-# LOAD-NEXT: cmdsize
-# LOAD-NEXT:    name /usr/lib/libc++.dylib
+# LOAD-NOT:    name /usr/lib/libc++.dylib
 # LOAD:          cmd LC_LOAD_DYLIB
----------------
this seems kind of sketch since this line could appear elsewhere in the output... `--implicit-check-not LC_LOAD_DYLIB` (like the other test is using) seems better


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

https://reviews.llvm.org/D103430