[PATCH] D68211: [clangd] Use the index-based API to do the header-source switch.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, mostly LG



================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:453
+void ClangdServer::switchSourceHeader(
+    PathRef Path, Callback<llvm::Optional<clangd::Path>> CB) {
+  if (auto CorrespondingFile =
----------------
could you add some comments explaining, why we first use file-only version(speed) and why try with an ast&index afterwards(making use of declarations)


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:195
 
   /// Helper function that returns a path to the corresponding source file when
   /// given a header file and vice versa.
----------------
i don't think it is a helper function anymore


================
Comment at: clang-tools-extra/clangd/unittests/HeaderSourceSwitchTests.cpp:250
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-I../include"}; // add search directory.
+  MockFSProvider FS;
----------------
could we rather provide `testPath("src/include")` to make sure this also works on windows?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68211