[clang-tools-extra] r342529 - [clangd] Store preamble macros in dynamic index.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Sep 19 02:35:04 2018
New Revision: 342529

URL: http://llvm.org/viewvc/llvm-project?rev=342529&view=rev
Log:
[clangd] Store preamble macros in dynamic index.

Summary:
Pros:
o Loading macros from preamble for every completion is slow (see profile).
o Calculating macro USR is also slow (see profile).
o Sema can provide a lot of macro completion results (e.g. when filter is empty,
60k for some large TUs!).

Cons:
o Slight memory increase in dynamic index (~1%).
o Some extra work during preamble build (should be fine as preamble build and
indexAST is way slower).

Before:
{F7195645}

After:
{F7195646}

Reviewers: ilya-biryukov, sammccall

Reviewed By: sammccall

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

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

Modified:
    clang-tools-extra/trunk/clangd/index/FileIndex.cpp
    clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
    clang-tools-extra/trunk/unittests/clangd/FileIndexTests.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=342529&r1=342528&r2=342529&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Wed Sep 19 02:35:04 2018
@@ -14,6 +14,7 @@
 #include "index/Index.h"
 #include "index/Merge.h"
 #include "clang/Index/IndexingAction.h"
+#include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include <memory>
 
@@ -41,10 +42,13 @@ indexSymbols(ASTContext &AST, std::share
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::DeclarationsOnly;
   IndexOpts.IndexFunctionLocals = false;
-
-  if (IsIndexMainAST)
+  if (IsIndexMainAST) {
     // We only collect refs when indexing main AST.
     CollectorOpts.RefFilter = RefKind::All;
+  }else {
+    IndexOpts.IndexMacrosInPreprocessor = true;
+    CollectorOpts.CollectMacro = true;
+  }
 
   SymbolCollector Collector(std::move(CollectorOpts));
   Collector.setPreprocessor(PP);

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=342529&r1=342528&r2=342529&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Wed Sep 19 02:35:04 2018
@@ -223,12 +223,13 @@ TEST(CompletionTest, Filter) {
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
       R"cpp(
-      #define MACRO X
-
       int global_var;
 
       int global_func();
 
+      // Make sure this is not in preamble.
+      #define MACRO X
+
       struct GlobalClass {};
 
       struct ClassWithMembers {
@@ -276,11 +277,12 @@ void TestAfterDotCompletion(clangd::Code
 void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
   auto Results = completions(
       R"cpp(
-      #define MACRO X
-
       int global_var;
       int global_func();
 
+      // Make sure this is not in preamble.
+      #define MACRO X
+
       struct GlobalClass {};
 
       struct ClassWithMembers {
@@ -430,10 +432,11 @@ TEST(CompletionTest, Snippets) {
 TEST(CompletionTest, Kinds) {
   auto Results = completions(
       R"cpp(
-          #define MACRO X
           int variable;
           struct Struct {};
           int function();
+          // make sure MACRO is not included in preamble.
+          #define MACRO 10
           int X = ^
       )cpp",
       {func("indexFunction"), var("indexVariable"), cls("indexClass")});
@@ -1921,6 +1924,21 @@ TEST(CompletionTest, MergeMacrosFromInde
               UnorderedElementsAre(Named("Clangd_Macro_Test")));
 }
 
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+  auto Results = completions(
+      R"cpp(#define CLANGD_PREAMBLE x
+
+          int x = 0;
+          #define CLANGD_MAIN x
+          void f() { CLANGD_^ }
+      )cpp",
+      {func("CLANGD_INDEX")});
+  // Index is overriden in code completion options, so the preamble symbol is
+  // not seen.
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+                                                        Named("CLANGD_INDEX")));
+}
+
 TEST(CompletionTest, DeprecatedResults) {
   std::string Body = R"cpp(
     void TestClangd();

Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=342529&r1=342528&r2=342529&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Wed Sep 19 02:35:04 2018
@@ -15,6 +15,7 @@
 #include "index/FileIndex.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "gtest/gtest.h"
@@ -330,6 +331,21 @@ TEST(FileIndexTest, Refs) {
                              FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, CollectMacros) {
+  FileIndex M;
+  update(M, "f", "#define CLANGD 1");
+
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  bool SeenSymbol = false;
+  M.index().fuzzyFind(Req, [&](const Symbol &Sym) {
+    EXPECT_EQ(Sym.Name, "CLANGD");
+    EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);
+    SeenSymbol = true;
+  });
+  EXPECT_TRUE(SeenSymbol);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang