[PATCH] D71029: [clangd] (take 2) Try harder to find a plausible `clang` as argv0, particularly on Mac.

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

The use of `xcrun` looks sound to me.



================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:53
+  }
+  StringRef Path = Buf->get()->getBuffer().trim();
+  if (Path.empty()) {
----------------
The `trim()` is probably safe - just wondering if that could cause issues in some weird case.


================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:65
+  if (llvm::sys::fs::real_path(Path, Resolved))
+    return Path; // On error;
+  return Resolved.str();
----------------
Just wondering why you decided to not somehow propagate the error - are there scenarios where the path is correct yet not resolvable on real FS?


================
Comment at: clang-tools-extra/clangd/CompileCommands.cpp:91
+      return resolve(std::move(*PathCC));
+  // Fallback: a nonexistent 'clang' binary next to clangd.
+  static int Dummy;
----------------
Maybe this warrants a mention in the header file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71029