[clang-tools-extra] r354792 - [clangd] Drop documentation in static index if symbols are not indexed for completion.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 25 08:00:00 PST 2019


Author: hokein
Date: Mon Feb 25 08:00:00 2019
New Revision: 354792

URL: http://llvm.org/viewvc/llvm-project?rev=354792&view=rev
Log:
[clangd] Drop documentation in static index if symbols are not indexed for completion.

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

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
    clang-tools-extra/trunk/clangd/index/FileIndex.cpp
    clang-tools-extra/trunk/clangd/index/IndexAction.cpp
    clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
    clang-tools-extra/trunk/clangd/index/SymbolCollector.h
    clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=354792&r1=354791&r2=354792&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Mon Feb 25 08:00:00 2019
@@ -45,9 +45,13 @@ indexSymbols(ASTContext &AST, std::share
   if (IsIndexMainAST) {
     // We only collect refs when indexing main AST.
     CollectorOpts.RefFilter = RefKind::All;
+    // Comments for main file can always be obtained from sema, do not store
+    // them in the index.
+    CollectorOpts.StoreAllDocumentation = false;
   } else {
     IndexOpts.IndexMacrosInPreprocessor = true;
     CollectorOpts.CollectMacro = true;
+    CollectorOpts.StoreAllDocumentation = true;
   }
 
   SymbolCollector Collector(std::move(CollectorOpts));

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=354792&r1=354791&r2=354792&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Mon Feb 25 08:00:00 2019
@@ -175,6 +175,7 @@ std::unique_ptr<FrontendAction> createSt
   Opts.CollectIncludePath = true;
   Opts.CountReferences = true;
   Opts.Origin = SymbolOrigin::Static;
+  Opts.StoreAllDocumentation = false;
   if (RefsCallback != nullptr) {
     Opts.RefFilter = RefKind::All;
     Opts.RefsInHeaders = true;

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=354792&r1=354791&r2=354792&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Feb 25 08:00:00 2019
@@ -566,18 +566,13 @@ const Symbol *SymbolCollector::addDeclar
   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);

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=354792&r1=354791&r2=354792&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Feb 25 08:00:00 2019
@@ -75,6 +75,10 @@ public:
     /// Collect symbols local to main-files, such as static functions
     /// and symbols inside an anonymous namespace.
     bool CollectMainFileSymbols = true;
+    /// 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;

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=354792&r1=354791&r2=354792&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Feb 25 08:00:00 2019
@@ -212,6 +212,12 @@ public:
         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;
@@ -710,6 +716,31 @@ TEST_F(SymbolCollectorTest, SymbolsInMai
                                    QName("main_f")));
 }
 
+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 {




More information about the cfe-commits mailing list