[PATCH] D140491: [lld-macho] Use ld64's LC_LINKER_OPTIONS behavior by default

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 22 15:23:55 PST 2022


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

lgtm



================
Comment at: lld/MachO/Driver.cpp:412
                        bool isReexport, bool isHidden, bool isExplicit,
-                       LoadType loadType) {
+                       LoadType loadType, InputFile *refLoc = nullptr) {
   if (std::optional<StringRef> path = findLibrary(name)) {
----------------
`refLoc` seems kind of confusing since we already have a `struct Location` that represents a file + offset. How about `refFile`, or `originFile`?


================
Comment at: lld/MachO/Driver.cpp:430
+    missingAutolinkWarnings.push_back(
+        saver().save((refLoc == nullptr ? "" : toString(refLoc) + ": ") +
+                     "auto-linked library not found for -l" + name));
----------------
personally I try to defer string building until we know we have to emit the warning. But I guess this code path occurs fairly rarely so there's not much of a perf concern (so this is fine)


================
Comment at: lld/test/MachO/lc-linker-option.ll:155-157
+; UNDEFINED_SYMBOL: undefined symbol: __SomeUndefinedSymbol
+; MISSING_AUTO_LINK_OPTION: {{.+}}load-missing.o: auto-linked framework not found for -framework Foo
+; MISSING_AUTO_LINK_OPTION: {{.+}}load-missing.o: auto-linked library not found for -lBar
----------------
could you hyphenate these too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140491



More information about the llvm-commits mailing list