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

[sammccall]

Summary

We can't resolve this (if it's a symlink) without further refactoring, but the
current behaviour is just incorrect.

[kadircet]

LGTM, thanks!

clang-tools-extra/clangd/CompileCommands.cpp

141

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))
 ...

[sammccall]

clang-tools-extra/clangd/CompileCommands.cpp

141

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.

[kadircet]

clang-tools-extra/clangd/CompileCommands.cpp

141

makes sense, i think we should land this soon-ish to stop the bleeding.

clang-tools-extra/clangd/CompileCommands.cpp

auto SiblingOf = [&](llvm::StringRef AbsPath) {
     llvm::SmallString<128> Result = llvm::sys::path::parent_path(AbsPath);
     llvm::sys::path::append(Result, llvm::sys::path::filename(Driver));
     return Result.str().str();
   };
 
   // First, eliminate relative paths.
   std::string Storage;
   if (!llvm::sys::path::is_absolute(Driver)) {
+    // If it's working-dir relative like bin/clang, we can't resolve it.
+    // FIXME: we could if we had the working directory here.
+    // Let's hope it's not a symlink.
+    if (llvm::any_of(Driver,
+                     [](char C) { return llvm::sys::path::is_separator(C); }))
+      return Driver.str();
     // If the driver is a generic like "g++" with no path, add clang dir.
     if (ClangPath &&
         (Driver == "clang" || Driver == "clang++" || Driver == "gcc" ||
          Driver == "g++" || Driver == "cc" || Driver == "c++")) {
       return SiblingOf(*ClangPath);
     }
     // Otherwise try to look it up on PATH. This won't change basename.
     auto Absolute = llvm::sys::findProgramByName(Driver);

clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

TEST(CommandMangler, ClangPath) {
   EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());
 
   Cmd = {testPath("path/clang++"), "foo.cc"};
   Mangler.adjust(Cmd);
   EXPECT_EQ(testPath("path/clang++"), Cmd.front());
 
   Cmd = {"foo/unknown-binary", "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());
+  EXPECT_EQ("foo/unknown-binary", Cmd.front());
 }
 
 // Only run the PATH/symlink resolving test on unix, we need to fiddle
 // with permissions and environment variables...
 #ifdef LLVM_ON_UNIX
 MATCHER(Ok, "") {
   if (arg) {
     *result_listener << arg.message();