[PATCH] D86910: [lld-macho] Implement and test resolution of common symbols

[Greg McGary via Phabricator via llvm-commits]

gkm added inline comments.


================
Comment at: lld/MachO/SymbolTable.cpp:85
     if (auto *common = dyn_cast<CommonSymbol>(s)) {
       if (size < common->size)
         return s;
----------------
s/`<`/`<=`/. I assume that `replaceSymbol` implies extra work, so why do that for equal-sized data?


================
Comment at: lld/MachO/SymbolTable.cpp:90-91
     }
+    // Common symbols take priority over all non-Defined symbols, so in case of
+    // a name conflict, we fall through to the replaceSymbol() call below.
   }
----------------
Tangent: I noticed that `DSOHandle` is not part of `SymbolUnion`. Should it be?
  


================
Comment at: lld/test/MachO/common-symbol-resolution.s:68-72
+#--- order
+## Order is important as we determine the size of a given symbol via the
+## address of the next symbol.
+_foo
+_foo_end
----------------
I don't see a use for this `order` file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86910