[clang-tools-extra] r373320 - [clangd] Use the index-based API to do the header-source switch.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 1 03:21:15 PDT 2019


Author: hokein
Date: Tue Oct  1 03:21:15 2019
New Revision: 373320

URL: http://llvm.org/viewvc/llvm-project?rev=373320&view=rev
Log:
[clangd] Use the index-based API to do the header-source switch.

Summary:
If the file heuristic fails, we try to use the index&AST to do the
header/source inference.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D68211

Modified:
    clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.cpp
    clang-tools-extra/trunk/clangd/ClangdServer.h
    clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
    clang-tools-extra/trunk/clangd/unittests/SyncAPI.cpp
    clang-tools-extra/trunk/clangd/unittests/SyncAPI.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Oct  1 03:21:15 2019
@@ -1038,10 +1038,16 @@ void ClangdLSPServer::onGoToDeclaration(
 void ClangdLSPServer::onSwitchSourceHeader(
     const TextDocumentIdentifier &Params,
     Callback<llvm::Optional<URIForFile>> Reply) {
-  if (auto Result = Server->switchSourceHeader(Params.uri.file()))
-    Reply(URIForFile::canonicalize(*Result, Params.uri.file()));
-  else
-    Reply(llvm::None);
+  Server->switchSourceHeader(
+      Params.uri.file(),
+      [Reply = std::move(Reply),
+       Params](llvm::Expected<llvm::Optional<clangd::Path>> Path) mutable {
+        if (!Path)
+          return Reply(Path.takeError());
+        if (*Path)
+          Reply(URIForFile::canonicalize(**Path, Params.uri.file()));
+        return Reply(llvm::None);
+      });
 }
 
 void ClangdLSPServer::onDocumentHighlight(

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Oct  1 03:21:15 2019
@@ -449,8 +449,24 @@ void ClangdServer::locateSymbolAt(PathRe
   WorkScheduler.runWithAST("Definitions", File, std::move(Action));
 }
 
-llvm::Optional<Path> ClangdServer::switchSourceHeader(PathRef Path) {
-  return getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem());
+void ClangdServer::switchSourceHeader(
+    PathRef Path, Callback<llvm::Optional<clangd::Path>> CB) {
+  // We want to return the result as fast as possible, stragety is:
+  //  1) use the file-only heuristic, it requires some IO but it is much
+  //     faster than building AST, but it only works when .h/.cc files are in
+  //     the same directory.
+  //  2) if 1) fails, we use the AST&Index approach, it is slower but supports
+  //     different code layout.
+  if (auto CorrespondingFile =
+          getCorrespondingHeaderOrSource(Path, FSProvider.getFileSystem()))
+    return CB(std::move(CorrespondingFile));
+  auto Action = [Path, CB = std::move(CB),
+                 this](llvm::Expected<InputsAndAST> InpAST) mutable {
+    if (!InpAST)
+      return CB(InpAST.takeError());
+    CB(getCorrespondingHeaderOrSource(Path, InpAST->AST, Index));
+  };
+  WorkScheduler.runWithAST("SwitchHeaderSource", Path, std::move(Action));
 }
 
 llvm::Expected<tooling::Replacements>

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Oct  1 03:21:15 2019
@@ -192,9 +192,10 @@ public:
   void locateSymbolAt(PathRef File, Position Pos,
                       Callback<std::vector<LocatedSymbol>> CB);
 
-  /// Helper function that returns a path to the corresponding source file when
-  /// given a header file and vice versa.
-  llvm::Optional<Path> switchSourceHeader(PathRef Path);
+  /// Switch to a corresponding source file when given a header file, and vice
+  /// versa.
+  void switchSourceHeader(PathRef Path,
+                          Callback<llvm::Optional<clangd::Path>> CB);
 
   /// Get document highlights for a given position.
   void findDocumentHighlights(PathRef File, Position Pos,

Modified: clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/HeaderSourceSwitchTests.cpp Tue Oct  1 03:21:15 2019
@@ -8,6 +8,7 @@
 
 #include "HeaderSourceSwitch.h"
 
+#include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -240,6 +241,32 @@ TEST(HeaderSourceSwitchTest, FromSourceT
   }
 }
 
+TEST(HeaderSourceSwitchTest, ClangdServerIntegration) {
+  class IgnoreDiagnostics : public DiagnosticsConsumer {
+    void onDiagnosticsReady(PathRef File,
+                            std::vector<Diag> Diagnostics) override {}
+  } DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {"-I" +
+                         testPath("src/include")}; // add search directory.
+  MockFSProvider FS;
+  // File heuristic fails here, we rely on the index to find the .h file.
+  std::string CppPath = testPath("src/lib/test.cpp");
+  std::string HeaderPath = testPath("src/include/test.h");
+  FS.Files[HeaderPath] = "void foo();";
+  const std::string FileContent = R"cpp(
+    #include "test.h"
+    void foo() {};
+  )cpp";
+  FS.Files[CppPath] = FileContent;
+  auto Options = ClangdServer::optsForTest();
+  Options.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Options);
+  runAddDocument(Server, CppPath, FileContent);
+  EXPECT_EQ(HeaderPath,
+            *llvm::cantFail(runSwitchHeaderSource(Server, CppPath)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/unittests/SyncAPI.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SyncAPI.cpp?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SyncAPI.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SyncAPI.cpp Tue Oct  1 03:21:15 2019
@@ -152,5 +152,12 @@ runSemanticRanges(ClangdServer &Server,
   return std::move(*Result);
 }
 
+llvm::Expected<llvm::Optional<clangd::Path>>
+runSwitchHeaderSource(ClangdServer &Server, PathRef File) {
+  llvm::Optional<llvm::Expected<llvm::Optional<clangd::Path>>> Result;
+  Server.switchSourceHeader(File, capture(Result));
+  return std::move(*Result);
+}
+
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/unittests/SyncAPI.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SyncAPI.h?rev=373320&r1=373319&r2=373320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SyncAPI.h (original)
+++ clang-tools-extra/trunk/clangd/unittests/SyncAPI.h Tue Oct  1 03:21:15 2019
@@ -56,6 +56,9 @@ RefSlab getRefs(const SymbolIndex &Index
 llvm::Expected<std::vector<Range>>
 runSemanticRanges(ClangdServer &Server, PathRef File, Position Pos);
 
+llvm::Expected<llvm::Optional<clangd::Path>>
+runSwitchHeaderSource(ClangdServer &Server, PathRef File);
+
 } // namespace clangd
 } // namespace clang
 




More information about the cfe-commits mailing list