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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:468
 
-  bool hasPathSep = (filename.find_first_of("/\\") != StringRef::npos);
-  if (hasPathSep)
+  if (sys::path::is_absolute(filename))
     return getFilename(filename);
----------------
thieta wrote:
> In order to get relative paths to work, I had to change this line. I am NOT sure what the original intent of this block was. It will make any relative path only return the filename. And none of the tests seems to cover this case, everything works after my change.
> 
> But if someone have a bit more context on this - I'll be happy to understand what's going on in this block.
I think the old logic was based on observed MSVC link.exe behavior.

Can you check that the new strategy is actually compatible with MSVC link.exe? As in, if we change clang to embed relative paths, does that actually work with MSVC link.exe with `-libpath:${resource}/lib`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151188