From e83caccb58cd360368fcda4381b63f6b43c84032 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Mon, 15 Oct 2018 11:46:26 +0000 Subject: [PATCH] [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 llvm-svn: 344507 --- clang-tools-extra/clangd/index/SymbolCollector.cpp | 36 +++++++-------- .../unittests/clangd/FileIndexTests.cpp | 51 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 26941d2..0d1281e 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -345,16 +345,20 @@ bool SymbolCollector::handleDeclOccurence( SM.getFileID(SpellingLoc) == SM.getMainFileID()) ReferencedDecls.insert(ND); - if ((static_cast(Opts.RefFilter) & Roles) && - SM.getFileID(SpellingLoc) == SM.getMainFileID()) - DeclRefs[ND].emplace_back(SpellingLoc, Roles); + bool CollectRef = static_cast(Opts.RefFilter) & Roles; + bool IsOnlyRef = + !(Roles & (static_cast(index::SymbolRole::Declaration) | + static_cast(index::SymbolRole::Definition))); - // Don't continue indexing if this is a mere reference. - if (!(Roles & static_cast(index::SymbolRole::Declaration) || - Roles & static_cast(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); } } } diff --git a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp index f526ec7..6f10953 100644 --- a/clang-tools-extra/unittests/clangd/FileIndexTests.cpp +++ b/clang-tools-extra/unittests/clangd/FileIndexTests.cpp @@ -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 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(), /*StoreInMemory=*/true, + [&](ASTContext &Ctx, std::shared_ptr PP) {}); + // Build AST for main file with preamble. + auto AST = ParsedAST::build( + createInvocationFromCommandLine(Cmd), PreambleData, + llvm::MemoryBuffer::getMemBufferCopy(Main.code()), + std::make_shared(), + 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 -- 2.7.4