[clang-tools-extra] r344507 - [clangd] Fix some references missing in dynamic index.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Mon Oct 15 04:46:26 2018
New Revision: 344507

URL: http://llvm.org/viewvc/llvm-project?rev=344507&view=rev
Log:
[clangd] Fix some references missing in dynamic index.

Summary:
Previously, SymbolCollector postfilters all references at the end to
find all references of interesting symbols.
It was incorrect when indxing main AST where we don't see locations
of symbol declarations and definitions in the main AST (as those are in
preamble AST).

The fix is to do earily check during collecting references.

Reviewers: sammccall

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

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

Modified:
    clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
    clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp

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=344507&r1=344506&r2=344507&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Oct 15 04:46:26 2018
@@ -345,16 +345,20 @@ bool SymbolCollector::handleDeclOccurenc
       SM.getFileID(SpellingLoc) == SM.getMainFileID())
     ReferencedDecls.insert(ND);
 
-  if ((static_cast<unsigned>(Opts.RefFilter) & Roles) &&
-      SM.getFileID(SpellingLoc) == SM.getMainFileID())
-    DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  bool CollectRef = static_cast<unsigned>(Opts.RefFilter) & Roles;
+  bool IsOnlyRef =
+      !(Roles & (static_cast<unsigned>(index::SymbolRole::Declaration) |
+                 static_cast<unsigned>(index::SymbolRole::Definition)));
 
-  // Don't continue indexing if this is a mere reference.
-  if (!(Roles & static_cast<unsigned>(index::SymbolRole::Declaration) ||
-        Roles & static_cast<unsigned>(index::SymbolRole::Definition)))
+  if (IsOnlyRef && !CollectRef)
     return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
     return true;
+  if (CollectRef && SM.getFileID(SpellingLoc) == SM.getMainFileID())
+    DeclRefs[ND].emplace_back(SpellingLoc, Roles);
+  // Don't continue indexing if this is a mere reference.
+  if (IsOnlyRef)
+    return true;
 
   auto ID = getSymbolID(ND);
   if (!ID)
@@ -476,17 +480,15 @@ void SymbolCollector::finish() {
     std::string MainURI = *MainFileURI;
     for (const auto &It : DeclRefs) {
       if (auto ID = getSymbolID(It.first)) {
-        if (Symbols.find(*ID)) {
-          for (const auto &LocAndRole : It.second) {
-            Ref R;
-            auto Range =
-                getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
-            R.Location.Start = Range.first;
-            R.Location.End = Range.second;
-            R.Location.FileURI = MainURI;
-            R.Kind = toRefKind(LocAndRole.second);
-            Refs.insert(*ID, R);
-          }
+        for (const auto &LocAndRole : It.second) {
+          Ref R;
+          auto Range =
+              getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());
+          R.Location.Start = Range.first;
+          R.Location.End = Range.second;
+          R.Location.FileURI = MainURI;
+          R.Kind = toRefKind(LocAndRole.second);
+          Refs.insert(*ID, R);
         }
       }
     }

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=344507&r1=344506&r2=344507&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Mon Oct 15 04:46:26 2018
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Annotations.h"
+#include "AST.h"
 #include "ClangdUnit.h"
 #include "TestFS.h"
 #include "TestTU.h"
@@ -15,6 +16,7 @@
 #include "index/FileIndex.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -346,6 +348,55 @@ TEST(FileIndexTest, CollectMacros) {
   EXPECT_TRUE(SeenSymbol);
 }
 
+TEST(FileIndexTest, ReferencesInMainFileWithPreamble) {
+  const std::string Header = R"cpp(
+    class Foo {};
+  )cpp";
+  Annotations Main(R"cpp(
+    #include "foo.h"
+    void f() {
+      [[Foo]] foo;
+    }
+  )cpp");
+  auto MainFile = testPath("foo.cpp");
+  auto HeaderFile = testPath("foo.h");
+  std::vector<const char*> Cmd = {"clang", "-xc++", MainFile.c_str()};
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = MainFile;
+  PI.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  PI.Contents = Main.code();
+  PI.FS = buildTestFS({{MainFile, Main.code()}, {HeaderFile, Header}});
+
+  // Prepare preamble.
+  auto CI = buildCompilerInvocation(PI);
+  auto PreambleData = buildPreamble(
+      MainFile,
+      *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
+      tooling::CompileCommand(), PI,
+      std::make_shared<PCHContainerOperations>(), /*StoreInMemory=*/true,
+      [&](ASTContext &Ctx, std::shared_ptr<Preprocessor> PP) {});
+  // Build AST for main file with preamble.
+  auto AST = ParsedAST::build(
+      createInvocationFromCommandLine(Cmd), PreambleData,
+      llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
+      std::make_shared<PCHContainerOperations>(),
+      PI.FS);
+  ASSERT_TRUE(AST);
+  FileIndex Index;
+  Index.updateMain(MainFile, *AST);
+
+  auto Foo =
+      findSymbol(TestTU::withHeaderCode(Header).headerSymbols(), "Foo");
+  RefsRequest Request;
+  Request.IDs.insert(Foo.ID);
+
+  // Expect to see references in main file, references in headers are excluded
+  // because we only index main AST.
+  EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())}));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang