[PATCH] D56539: [clangd] Drop documentation in static index if symbols are not indexed for completion.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.

This is a further optimization of r350803, we drop docs in static index for
symbols not being indexed for completion, while keeping the docs in dynamic
index (we rely on dynamic index to get docs for class members).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56539

Files:
  clangd/index/FileIndex.cpp
  clangd/index/IndexAction.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -202,6 +202,12 @@
         return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
       }
 
+      bool BeginInvocation(CompilerInstance &CI) override {
+        // Make the compiler parse all comments.
+        CI.getLangOpts().CommentOpts.ParseAllComments = true;
+        return WrapperFrontendAction::BeginInvocation(CI);
+      }
+
     private:
       index::IndexingOptions IndexOpts;
       CommentHandler *PragmaHandler;
@@ -639,6 +645,34 @@
               UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
 }
 
+TEST_F(SymbolCollectorTest, Documentation) {
+  const std::string Header = R"(
+    // Doc Foo
+    class Foo {
+      // Doc f
+      int f();
+    };
+  )";
+  {
+    CollectorOpts.StoreAllDocumentation = false;
+    runSymbolCollector(Header, /* Main */ "");
+    EXPECT_THAT(Symbols, UnorderedElementsAre(
+                             AllOf(QName("Foo"), Doc("Doc Foo"),
+                                   ForCodeCompletion(true)),
+                             AllOf(QName("Foo::f"), Doc(""), ReturnType(""),
+                                   ForCodeCompletion(false))));
+  }
+  {
+    CollectorOpts.StoreAllDocumentation = true;
+    runSymbolCollector(Header, /* Main */ "");
+    EXPECT_THAT(Symbols, UnorderedElementsAre(
+                             AllOf(QName("Foo"), Doc("Doc Foo"),
+                                   ForCodeCompletion(true)),
+                             AllOf(QName("Foo::f"), Doc("Doc f"),
+                                   ReturnType(""), ForCodeCompletion(false))));
+  }
+}
+
 TEST_F(SymbolCollectorTest, ClassMembers) {
   const std::string Header = R"(
     class Foo {
Index: clangd/index/SymbolCollector.h
===================================================================
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -72,6 +72,10 @@
     /// collect macros. For example, `indexTopLevelDecls` will not index any
     /// macro even if this is true.
     bool CollectMacro = false;
+    /// If set to true, SymbolCollector will collect doc for all symbols.
+    /// Note that documents of symbols being indexed for completion will always
+    /// be collected regardless of this option.
+    bool StoreAllDocumentation = false;
     /// If this is set, only collect symbols/references from a file if
     /// `FileFilter(SM, FID)` is true. If not set, all files are indexed.
     std::function<bool(const SourceManager &, FileID)> FileFilter = nullptr;
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -546,18 +546,13 @@
   std::string Documentation =
       formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
                                               /*CommentsFromHeaders=*/true));
-  // For symbols not indexed for completion (class members), we also store their
-  // docs in the index, because Sema doesn't load the docs from the preamble, we
-  // rely on the index to get the docs.
-  // FIXME: this can be optimized by only storing the docs in dynamic index --
-  // dynamic index should index these symbols when Sema completes a member
-  // completion.
-  S.Documentation = Documentation;
   if (!(S.Flags & Symbol::IndexedForCodeCompletion)) {
+    if (Opts.StoreAllDocumentation)
+      S.Documentation = Documentation;
     Symbols.insert(S);
     return Symbols.find(S.ID);
   }
-
+  S.Documentation = Documentation;
   std::string Signature;
   std::string SnippetSuffix;
   getSignature(*CCS, &Signature, &SnippetSuffix);
Index: clangd/index/IndexAction.cpp
===================================================================
--- clangd/index/IndexAction.cpp
+++ clangd/index/IndexAction.cpp
@@ -175,6 +175,7 @@
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
   Opts.Origin = SymbolOrigin::Static;
+  Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
     Opts.RefFilter = RefKind::All;
     Opts.RefsInHeaders = true;
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -48,9 +48,11 @@
   if (IsIndexMainAST) {
     // We only collect refs when indexing main AST.
     CollectorOpts.RefFilter = RefKind::All;
-  }else {
+    CollectorOpts.StoreAllDocumentation = false;
+  } else {
     IndexOpts.IndexMacrosInPreprocessor = true;
     CollectorOpts.CollectMacro = true;
+    CollectorOpts.StoreAllDocumentation = true;
   }
 
   SymbolCollector Collector(std::move(CollectorOpts));

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits