[PATCH] D102143: [lld-macho] Treat undefined symbols uniformly

[Nico Weber via Phabricator via llvm-commits]

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

lg with writer.cpp change undone



================
Comment at: lld/MachO/SymbolTable.cpp:190
+void lld::macho::treatUndefinedSymbol(const Undefined &sym, StringRef source) {
+  auto message = [&](const Undefined &sym) {
+    std::string fileName;
----------------
int3 wrote:
> thakis wrote:
> > nit: s/&/source/
> > 
> > And if we're doing capturing, do `[source, sym]` and remove the arg, to make things self-consistent.
> > 
> > (`&` works here, but with `&` it's easy to accidentally capture a stack variable, and if the closure escapes that's a recipe for subtle bugs, and compilers don't yet warn on default-capturing locals in escaping closures. So I try to not use default capture.)
> fair enough. while at it I renamed `message` to `msg` so it doesn't shadow the `message` defined in `lld/Common`.
meh, the local `message` string shadows too. i'd keep the old name (but up to you)


================
Comment at: lld/MachO/SymbolTable.cpp:194
+      return message + "\n>>> referenced by " + source.str();
+    else
+      return message + "\n>>> referenced by " + toString(sym.getFile());
----------------
Since you're doing `return`s in the conditional now: style guide says no else after return :P


================
Comment at: lld/MachO/Writer.cpp:1061
+      treatUndefinedSymbol(*undefined, "the entry point");
+    if (!isa<Undefined>(config->entry))
+      prepareBranchTarget(config->entry);
----------------
...this change is no longer needed now that this is done in driver, rigth?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102143