[PATCH] D93263: [lld-macho] Implement option: -undefined TREATMENT

[Greg McGary via Phabricator via llvm-commits]

gkm marked 6 inline comments as done.
gkm added inline comments.


================
Comment at: lld/MachO/SymbolTable.cpp:163
+  default:
+    fatal("malformed -undefined treatment");
+    return false;
----------------
int3 wrote:
> with the change suggested above, this could be `llvm_unreachable` instead of `fatal`
I dropped `default:` entirely, because clang warns that it is unused as every enum case is explicitly handled.


================
Comment at: lld/MachO/SymbolTable.h:58
 
+extern bool undefinedSymbol(const std::string symbolName,
+                            const std::string referenceName);
----------------
int3 wrote:
> I think this function is a verb-y function but the current name is noun-ish. How about naming it `treatUndefinedSymbol`? (ELF/COFF use `reportUndefinedSymbol`, but since we're not always going to report it...)
Since the API differs, I chose `treatUndefinedSymbol()`


================
Comment at: lld/MachO/Writer.cpp:414
         if (isa<Undefined>(s))
-          error("undefined symbol " + toString(*s) + ", referenced from " +
-                toString(isec->file));
+          undefinedSymbol(toString(*s), toString(isec->file));
         else
----------------
int3 wrote:
> doesn't seem like `undefinedSymbol`'s return value is used anywhere. Can it just be a `void` function?
The `bool` was intended to accommodate use for the `config->outputType == MH_EXECUTE` case in `lld::macho::link()`, but I think it should always `error()` there. I made it `void`, since that intention was unrealized. We can always revise to return `bool` if/when we have a real use for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93263