[PATCH] D100147: [lld-macho] Re-root absolute input file paths if -syslibroot is specified

[Jez Ng via Phabricator via llvm-commits]

int3 marked 3 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:80
+// Search for all possible combinations of `{root}/{name}.{extension}`.
+// If \p extensions are not specified, then just search for `{root}/{name}`.
+static Optional<StringRef>
----------------
gkm wrote:
> What does `\p` mean here in this comment?
Parameter. It's used by Doxygen: https://www.doxygen.nl/manual/commands.html#cmdp


================
Comment at: lld/test/MachO/reroot-path.s:26-27
+## Test our various file-loading flags to make sure all bases are covered.
+# RUN: %lld -lSystem -syslibroot %t %t/foo.a %t/%:t/bar.a %t/test.o -o /dev/null
+# RUN: %lld -lSystem -syslibroot %t -force_load %t/foo.a -force_load %t/%:t/bar.a %t/test.o -o /dev/null
+# RUN: %lld -lSystem -syslibroot %t %t/libfoo.dylib %t/%:t/libbar.dylib %t/test.o -o /dev/null
----------------
gkm wrote:
> gkm wrote:
> > These don't test what you want, since they succeed even without `-syslibroot`:
> > * PASS without `-syslibroot %t`, since `%t/%:t/bar.a` is a real path, so the syslibroot is unused
> > * PASS with `-syslibroot %t` and s/`%t/%:t/bar.a`/`%t/bar.a`/ for a syslibroot-relative path
> > * FAIL (as expected) without `-syslibroot %t` and s/`%t/%:t/bar.a`/`%t/bar.a`/, since syslibroot is needed to reach the syslibroot-relative path
> I believe the second case is what you want: with `-syslibroot %t` and s/`%t/%:t/bar.a`/`%t/bar.a`/
yea good catch. I'll check the actual path accessed with `-t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100147