[PATCH] D151188: [LLD][COFF] Add LLVM toolchain library paths by default.

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

Nice!



================
Comment at: lld/COFF/Driver.cpp:650
+  sys::path::append(libDir, "lib");
+  searchPaths.insert(searchPaths.begin(), saver().save(std::string(libDir)));
+
----------------
Instead of constructing a `std::string`, this could be `libDir.str()`. Same for the others below.


================
Comment at: lld/COFF/Driver.cpp:1569
 
+  addClangLibSearchPaths(argsArr[0]);
+
----------------
nit: I'd suggest maybe skipping the blank line above, to keep this within the "Construct search path list" block.


================
Comment at: lld/test/COFF/print-search-paths.s:3
 # RUN: llvm-mc -triple x86_64-windows-msvc %s -filetype=obj -o %t.obj
-# RUN: lld-link -safeseh:no /dll /noentry /winsysroot:%t.dir/sysroot /vctoolsversion:1.1.1.1 /winsdkversion:10.0.1 %t.obj -print-search-paths | FileCheck -DSYSROOT=%t.dir %s
+# RUN: lld-link -safeseh:no /dll /noentry /winsysroot:%t.dir/sysroot /vctoolsversion:1.1.1.1 /winsdkversion:10.0.1 %t.obj -print-search-paths | sort | FileCheck -DSYSROOT=%t.dir %s
 # RUN: llvm-mc -triple i686-windows-msvc %s -filetype=obj -o %t_32.obj
----------------
I suppose the `sort` makes the output nicer, but doesn't it the test can't tell the difference between e.g. append or prepending the new paths?

(Also ending up with `Library search paths:` on the last line instead of the first looks funny.)


================
Comment at: lld/test/COFF/print-search-paths.s:10
 # CHECK:   [[SYSROOT]]{{[/\\]}}sysroot{{[/\\]}}Windows Kits{{[/\\]}}10{{[/\\]}}Lib{{[/\\]}}10.0.1{{[/\\]}}um{{[/\\]}}x64
-# X86: Library search paths:
-# X86:   [[SYSROOT]]{{[/\\]}}sysroot{{[/\\]}}VC{{[/\\]}}Tools{{[/\\]}}MSVC{{[/\\]}}1.1.1.1{{[/\\]}}lib{{[/\\]}}x86
+# CHECK:   lib{{[/\\]}}clang{{[/\\]}}{{[0-9]+}}{{[/\\]}}lib
+# CHECK:   lib{{[/\\]}}clang{{[/\\]}}{{[0-9]+}}{{[/\\]}}lib{{[/\\]}}windows
----------------
Is the `lib/` dir (without the `clang/`) missing?


================
Comment at: lld/test/COFF/print-search-paths.s:10
 # CHECK:   [[SYSROOT]]{{[/\\]}}sysroot{{[/\\]}}Windows Kits{{[/\\]}}10{{[/\\]}}Lib{{[/\\]}}10.0.1{{[/\\]}}um{{[/\\]}}x64
-# X86: Library search paths:
-# X86:   [[SYSROOT]]{{[/\\]}}sysroot{{[/\\]}}VC{{[/\\]}}Tools{{[/\\]}}MSVC{{[/\\]}}1.1.1.1{{[/\\]}}lib{{[/\\]}}x86
+# CHECK:   lib{{[/\\]}}clang{{[/\\]}}{{[0-9]+}}{{[/\\]}}lib
+# CHECK:   lib{{[/\\]}}clang{{[/\\]}}{{[0-9]+}}{{[/\\]}}lib{{[/\\]}}windows
----------------
thieta wrote:
> I would like to check the full path here similar to the `[[SYSROOT]]` stuff, but I couldn't find a substitution that gives me the binary/build directory path. Does anyone have a hint or another idea how to test that?
I don't have any good ideas for that, but you could at least use a regex to capture the "base path" for the first dir and then re-use that named regex for the next lines, to ensure that they're the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151188