[PATCH] D117354: [lld-macho] Allow order files and call graph sorting to be used together

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/MachO/SectionPriorities.cpp:38
+
+size_t kLowestPriority = std::numeric_limits<size_t>::max();
+
----------------
it's confusing that this variable is named with `k`, which I assume means it's a constant, and yet it's updated by the function below. 

(also I think this is a google specific style - the LLVM coding standard seems to suggest camel case only)


================
Comment at: lld/MachO/SectionPriorities.cpp:253
 
+static Optional<size_t> getSymbolPriority(Defined& sym) {
+    if (sym.isAbsolute())
----------------
you're not modifying the symbol


================
Comment at: lld/MachO/SectionPriorities.cpp:261
+    const SymbolPriorityEntry &entry = it->second;
+    const InputFile *f = sym.isec->getFile();
+    // We don't use toString(InputFile *) here because it returns the full path
----------------
You may need to check this against NULL since synthetic symbols don't have InputFile (they'd return NULL)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117354