[PATCH] D82011: [clangd] Don't mangle workdir-relevant driver path in compile commands

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:141
+    // Let's hope it's not a symlink.
+    if (llvm::any_of(Driver,
+                     [](char C) { return llvm::sys::path::is_separator(C); }))
----------------
kadircet wrote:
> I believe it would be clearer if you put it below, into `else if (ClangPath)` part.
> 
> i.e.
> ```
> if (Absolute ...)
>   ..
> // If we couldn't find program and driver is just a filename, use clang dir
> // FIXME: Note that Driver can still be relative to workdir even if it doesn't have any path separators.
> // We should pass WD into here and try to make Driver absolute.
> else if(ClangPath && !hasPathSeparators(Driver))
>  ...
> ```
This would rely on the undocumented behaviour of findProgramByName with workdir-relative paths, which I think would be nice to avoid (for clarity to the reader if nothing else).
Doing (what looks like) a PATH search for a workdir-relative path is just conceptually wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82011