hokein created this revision. hokein added a reviewer: sammccall. Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.
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. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53273 Files: clangd/index/SymbolCollector.cpp unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp =================================================================== --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -8,13 +8,15 @@ //===----------------------------------------------------------------------===// #include "Annotations.h" +#include "AST.h" #include "ClangdUnit.h" #include "TestFS.h" #include "TestTU.h" #include "gmock/gmock.h" #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 @@ 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 Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -345,16 +345,17 @@ SM.getFileID(SpellingLoc) == SM.getMainFileID()) ReferencedDecls.insert(ND); + if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) + return true; + if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && SM.getFileID(SpellingLoc) == SM.getMainFileID()) DeclRefs[ND].emplace_back(SpellingLoc, Roles); // 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))) return true; - if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) - return true; auto ID = getSymbolID(ND); if (!ID) @@ -476,17 +477,15 @@ 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); } } }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits