[PATCH] D158003: [lld-macho] Stricter Bitcode Symbol Resolution

[Vincent Lee via Phabricator via llvm-commits]

thevinster accepted this revision.
thevinster added a comment.
This revision is now accepted and ready to land.

Overall, lgtm. Minor comments



================
Comment at: lld/MachO/SymbolTable.cpp:155
+        auto objFile = dyn_cast<ObjFile>(file);
+        if (!objFile) {
+          // The file must be a native object file, as opposed to potentially
----------------
Can we also add a condition to cast this file as a bitcode file just make sure that it is in fact a bitcode file? 


================
Comment at: lld/MachO/SymbolTable.cpp:156-166
+          // The file must be a native object file, as opposed to potentially
+          // being another bitcode file. A situation arises when some symbols
+          // are defined thru `module asm` and thus they are not present in the
+          // bitcode's symbol table. Consider bitcode modules `A`, `B`, and `C`,
+          // LTO compiles only `A` and `C`, since there's no explicit symbol
+          // reference to `B` other than a symbol from `A` via `module asm`.
+          // After LTO is finished, the missing symbol now appears in the
----------------
So if I understand the comment here correctly, A has a `module asm` symbol that is defined in B, but incorrectly resolves it to the bitcode file instead of the object file? This dependency issue kinda seems like a bug rather than a limitation and I wonder if this scenario should be supported. Though using `module asm` in source is already sketchy, in itself, so it might be worth error-ing to as a forcing function to make users fix their errors at the right level. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158003