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

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added a comment.

LG - thanks!
(will leave it open for thakis)



================
Comment at: lld/MachO/SectionPriorities.cpp:371
+
+  auto addSym = [&](Defined &sym) {
+    Optional<size_t> symbolPriority = getSymbolPriority(&sym);
----------------
i would suggest changing this to `const Defined* sym` too


================
Comment at: lld/MachO/SectionPriorities.cpp:379
+
+  // TODO: Make sure this handles weak symbols correctly.
+  for (const InputFile *file : inputFiles) {
----------------
nit: unless you're planning on taking care of this shortly - a bit more info on what to be done here would be helpful


================
Comment at: lld/MachO/SectionPriorities.cpp:253
 
+static Optional<size_t> getSymbolPriority(Defined& sym) {
+    if (sym.isAbsolute())
----------------
lgrey wrote:
> oontvoo wrote:
> > you're not modifying the symbol
> Done (but for my edification: why pointer to const vs. const ref?)
(commented below also - I suggested const* because the objects themselves are stored as pointers, so you can avoid the whole deref + taking address trip. Also it's not common to see const ref in lld code)


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

https://reviews.llvm.org/D117354