Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -13,6 +13,7 @@ #include "Headers.h" #include "SourceCode.h" #include "Trace.h" +#include "index/CanonicalIncludes.h" #include "index/FileIndex.h" #include "index/Merge.h" #include "refactor/Tweak.h" @@ -69,9 +70,10 @@ : FIndex(FIndex), DiagConsumer(DiagConsumer) {} void onPreambleAST(PathRef Path, ASTContext &Ctx, - std::shared_ptr PP) override { + std::shared_ptr PP, + const CanonicalIncludes &CanonIncludes) override { if (FIndex) - FIndex->updatePreamble(Path, Ctx, std::move(PP)); + FIndex->updatePreamble(Path, Ctx, std::move(PP), CanonIncludes); } void onMainAST(PathRef Path, ParsedAST &AST) override { Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -16,6 +16,7 @@ #include "Headers.h" #include "Path.h" #include "Protocol.h" +#include "index/CanonicalIncludes.h" #include "index/Index.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/PrecompiledPreamble.h" @@ -48,7 +49,8 @@ struct PreambleData { PreambleData(PrecompiledPreamble Preamble, std::vector Diags, IncludeStructure Includes, - std::unique_ptr StatCache); + std::unique_ptr StatCache, + CanonicalIncludes CanonIncludes); tooling::CompileCommand CompileCommand; PrecompiledPreamble Preamble; @@ -59,6 +61,7 @@ // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. std::unique_ptr StatCache; + CanonicalIncludes CanonIncludes; }; /// Stores and provides access to parsed AST. @@ -100,13 +103,14 @@ /// bytes. Does not include the size of the preamble. std::size_t getUsedBytes() const; const IncludeStructure &getIncludeStructure() const; + const CanonicalIncludes &getCanonicalIncludes() const; private: ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, std::vector LocalTopLevelDecls, std::vector Diags, - IncludeStructure Includes); + IncludeStructure Includes, CanonicalIncludes CanonIncludes); // In-memory preambles must outlive the AST, it is important that this member // goes before Clang and Action. @@ -125,10 +129,12 @@ // top-level decls from the preamble. std::vector LocalTopLevelDecls; IncludeStructure Includes; + CanonicalIncludes CanonIncludes; }; using PreambleParsedCallback = - std::function)>; + std::function, + const CanonicalIncludes &)>; /// Rebuild the preamble for the new inputs unless the old one can be reused. /// If \p OldPreamble can be reused, it is returned unchanged. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -16,6 +16,7 @@ #include "Logger.h" #include "SourceCode.h" #include "Trace.h" +#include "index/CanonicalIncludes.h" #include "index/Index.h" #include "clang/AST/ASTContext.h" #include "clang/Basic/LangOptions.h" @@ -99,11 +100,16 @@ IncludeStructure takeIncludes() { return std::move(Includes); } + CanonicalIncludes takeCanonicalIncludes() { + addSystemHeadersMapping(&CanonIncludes); + return std::move(CanonIncludes); + } + void AfterExecute(CompilerInstance &CI) override { if (!ParsedCallback) return; trace::Span Tracer("Running PreambleCallback"); - ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr()); + ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(), CanonIncludes); } void BeforeExecute(CompilerInstance &CI) override { @@ -115,10 +121,17 @@ return collectIncludeStructureCallback(*SourceMgr, &Includes); } + CommentHandler *getCommentHandler() override { + IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes); + return IWYUHandler.get(); + } + private: PathRef File; PreambleParsedCallback ParsedCallback; IncludeStructure Includes; + CanonicalIncludes CanonIncludes; + std::unique_ptr IWYUHandler = nullptr; SourceManager *SourceMgr = nullptr; }; @@ -324,6 +337,17 @@ Clang->getPreprocessor().addPPCallbacks( collectIncludeStructureCallback(Clang->getSourceManager(), &Includes)); + // Copy over the includes from the preamble, then combine with the + // non-preamble includes below. + CanonicalIncludes CanonIncludes; + if (Preamble) + CanonIncludes = Preamble->CanonIncludes; + else + addSystemHeadersMapping(&CanonIncludes); + std::unique_ptr IWYUHandler = + collectIWYUHeaderMaps(&CanonIncludes); + Clang->getPreprocessor().addCommentHandler(IWYUHandler.get()); + if (!Action->Execute()) log("Execute() failed when building AST for {0}", MainInput.getFile()); @@ -353,7 +377,7 @@ Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end()); return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action), std::move(ParsedDecls), std::move(Diags), - std::move(Includes)); + std::move(Includes), std::move(CanonIncludes)); } ParsedAST::ParsedAST(ParsedAST &&Other) = default; @@ -429,21 +453,28 @@ return Includes; } +const CanonicalIncludes &ParsedAST::getCanonicalIncludes() const { + return CanonIncludes; +} + PreambleData::PreambleData(PrecompiledPreamble Preamble, std::vector Diags, IncludeStructure Includes, - std::unique_ptr StatCache) + std::unique_ptr StatCache, + CanonicalIncludes CanonIncludes) : Preamble(std::move(Preamble)), Diags(std::move(Diags)), - Includes(std::move(Includes)), StatCache(std::move(StatCache)) {} + Includes(std::move(Includes)), StatCache(std::move(StatCache)), + CanonIncludes(std::move(CanonIncludes)) {} ParsedAST::ParsedAST(std::shared_ptr Preamble, std::unique_ptr Clang, std::unique_ptr Action, std::vector LocalTopLevelDecls, - std::vector Diags, IncludeStructure Includes) + std::vector Diags, IncludeStructure Includes, + CanonicalIncludes CanonIncludes) : Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), LocalTopLevelDecls(std::move(LocalTopLevelDecls)), - Includes(std::move(Includes)) { + Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) { assert(this->Clang); assert(this->Action); } @@ -510,7 +541,8 @@ FileName); return std::make_shared( std::move(*BuiltPreamble), PreambleDiagnostics.take(), - SerializedDeclsCollector.takeIncludes(), std::move(StatCache)); + SerializedDeclsCollector.takeIncludes(), std::move(StatCache), + SerializedDeclsCollector.takeCanonicalIncludes()); } else { elog("Could not build a preamble for file {0}", FileName); return nullptr; Index: clangd/TUScheduler.h =================================================================== --- clangd/TUScheduler.h +++ clangd/TUScheduler.h @@ -12,6 +12,7 @@ #include "ClangdUnit.h" #include "Function.h" #include "Threading.h" +#include "index/CanonicalIncludes.h" #include "llvm/ADT/StringMap.h" #include @@ -91,7 +92,8 @@ /// contains only AST nodes from the #include directives at the start of the /// file. AST node in the current file should be observed on onMainAST call. virtual void onPreambleAST(PathRef Path, ASTContext &Ctx, - std::shared_ptr PP) {} + std::shared_ptr PP, + const CanonicalIncludes &) {} /// Called on the AST built for the file itself. Note that preamble AST nodes /// are not deserialized and should be processed in the onPreambleAST call /// instead. Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -45,6 +45,7 @@ #include "Cancellation.h" #include "Logger.h" #include "Trace.h" +#include "index/CanonicalIncludes.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "llvm/ADT/ScopeExit.h" @@ -385,8 +386,9 @@ std::shared_ptr NewPreamble = buildPreamble( FileName, *Invocation, OldPreamble, OldCommand, Inputs, PCHs, StorePreambleInMemory, - [this](ASTContext &Ctx, std::shared_ptr PP) { - Callbacks.onPreambleAST(FileName, Ctx, std::move(PP)); + [this](ASTContext &Ctx, std::shared_ptr PP, + const CanonicalIncludes &CanonIncludes) { + Callbacks.onPreambleAST(FileName, Ctx, std::move(PP), CanonIncludes); }); bool CanReuseAST = InputsAreTheSame && (OldPreamble == NewPreamble); Index: clangd/index/FileIndex.h =================================================================== --- clangd/index/FileIndex.h +++ clangd/index/FileIndex.h @@ -19,6 +19,7 @@ #include "Index.h" #include "MemIndex.h" #include "Merge.h" +#include "index/CanonicalIncludes.h" #include "clang/Lex/Preprocessor.h" #include @@ -84,7 +85,8 @@ /// Update preamble symbols of file \p Path with all declarations in \p AST /// and macros in \p PP. void updatePreamble(PathRef Path, ASTContext &AST, - std::shared_ptr PP); + std::shared_ptr PP, + const CanonicalIncludes &Includes); /// Update symbols and references from main file \p Path with /// `indexMainDecls`. @@ -124,8 +126,8 @@ /// Idex declarations from \p AST and macros from \p PP that are declared in /// included headers. -SymbolSlab indexHeaderSymbols(ASTContext &AST, - std::shared_ptr PP); +SymbolSlab indexHeaderSymbols(ASTContext &AST, std::shared_ptr PP, + const CanonicalIncludes &Includes); } // namespace clangd } // namespace clang Index: clangd/index/FileIndex.cpp =================================================================== --- clangd/index/FileIndex.cpp +++ clangd/index/FileIndex.cpp @@ -10,6 +10,7 @@ #include "ClangdUnit.h" #include "Logger.h" #include "SymbolCollector.h" +#include "index/CanonicalIncludes.h" #include "index/Index.h" #include "index/MemIndex.h" #include "index/Merge.h" @@ -28,14 +29,11 @@ static std::pair indexSymbols(ASTContext &AST, std::shared_ptr PP, - llvm::ArrayRef DeclsToIndex, bool IsIndexMainAST) { + llvm::ArrayRef DeclsToIndex, + const CanonicalIncludes &Includes, bool IsIndexMainAST) { SymbolCollector::Options CollectorOpts; - // FIXME(ioeric): we might also want to collect include headers. We would need - // to make sure all includes are canonicalized (with CanonicalIncludes), which - // is not trivial given the current way of collecting symbols: we only have - // AST at this point, but we also need preprocessor callbacks (e.g. - // CommentHandler for IWYU pragma) to canonicalize includes. - CollectorOpts.CollectIncludePath = false; + CollectorOpts.CollectIncludePath = true; + CollectorOpts.Includes = &Includes; CollectorOpts.CountReferences = false; CollectorOpts.Origin = SymbolOrigin::Dynamic; @@ -47,7 +45,7 @@ if (IsIndexMainAST) { // We only collect refs when indexing main AST. CollectorOpts.RefFilter = RefKind::All; - }else { + } else { IndexOpts.IndexMacrosInPreprocessor = true; CollectorOpts.CollectMacro = true; } @@ -72,16 +70,16 @@ std::pair indexMainDecls(ParsedAST &AST) { return indexSymbols(AST.getASTContext(), AST.getPreprocessorPtr(), - AST.getLocalTopLevelDecls(), + AST.getLocalTopLevelDecls(), AST.getCanonicalIncludes(), /*IsIndexMainAST=*/true); } -SymbolSlab indexHeaderSymbols(ASTContext &AST, - std::shared_ptr PP) { +SymbolSlab indexHeaderSymbols(ASTContext &AST, std::shared_ptr PP, + const CanonicalIncludes &Includes) { std::vector DeclsToIndex( AST.getTranslationUnitDecl()->decls().begin(), AST.getTranslationUnitDecl()->decls().end()); - return indexSymbols(AST, std::move(PP), DeclsToIndex, + return indexSymbols(AST, std::move(PP), DeclsToIndex, Includes, /*IsIndexMainAST=*/false) .first; } @@ -195,8 +193,9 @@ MainFileIndex(llvm::make_unique()) {} void FileIndex::updatePreamble(PathRef Path, ASTContext &AST, - std::shared_ptr PP) { - auto Symbols = indexHeaderSymbols(AST, std::move(PP)); + std::shared_ptr PP, + const CanonicalIncludes &Includes) { + auto Symbols = indexHeaderSymbols(AST, std::move(PP), Includes); PreambleSymbols.update(Path, llvm::make_unique(std::move(Symbols)), llvm::make_unique()); Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -645,6 +645,36 @@ EXPECT_THAT(Results, ElementsAre(Named("ifndef"))); } +TEST(CompletionTest, DynamicIndexIncludeInsertion) { + MockFSProvider FS; + MockCompilationDatabase CDB; + IgnoreDiagnostics DiagConsumer; + ClangdServer::Options Opts = ClangdServer::optsForTest(); + Opts.BuildDynamicSymbolIndex = true; + ClangdServer Server(CDB, FS, DiagConsumer, Opts); + + FS.Files[testPath("foo_header.h")] = R"cpp( + struct Foo { + // Member doc + int foo(); + }; + )cpp"; + const std::string FileContent(R"cpp( + #include "foo_header.h" + int Foo::foo() { + return 42; + } + )cpp"); + Server.addDocument(testPath("foo_impl.cpp"), FileContent); + // Wait for the dynamic index being built. + ASSERT_TRUE(Server.blockUntilIdleForTest()); + EXPECT_THAT( + completions(Server, "Foo^ foo;").Completions, + ElementsAre(AllOf(Named("Foo"), + HasInclude('"' + testPath("foo_header.h") + '"'), + InsertInclude()))); +} + TEST(CompletionTest, DynamicIndexMultiFile) { MockFSProvider FS; MockCompilationDatabase CDB; Index: unittests/clangd/FileIndexTests.cpp =================================================================== --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -12,6 +12,7 @@ #include "SyncAPI.h" #include "TestFS.h" #include "TestTU.h" +#include "index/CanonicalIncludes.h" #include "index/FileIndex.h" #include "index/Index.h" #include "clang/Frontend/CompilerInvocation.h" @@ -147,8 +148,8 @@ File.HeaderFilename = (Basename + ".h").str(); File.HeaderCode = Code; auto AST = File.build(); - M.updatePreamble(File.Filename, AST.getASTContext(), - AST.getPreprocessorPtr()); + M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(), + AST.getCanonicalIncludes()); } TEST(FileIndexTest, CustomizedURIScheme) { @@ -199,13 +200,16 @@ QName("X::f"))); } -TEST(FileIndexTest, NoIncludeCollected) { +TEST(FileIndexTest, IncludeCollected) { FileIndex M; - update(M, "f", "class string {};"); + update( + M, "f", + "// IWYU pragma: private, include \nclass string {};"); auto Symbols = runFuzzyFind(M, ""); EXPECT_THAT(Symbols, ElementsAre(_)); - EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty()); + EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader, + ""); } TEST(FileIndexTest, TemplateParamsInLabel) { @@ -270,10 +274,11 @@ buildPreamble( FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, std::make_shared(), /*StoreInMemory=*/true, - [&](ASTContext &Ctx, std::shared_ptr PP) { + [&](ASTContext &Ctx, std::shared_ptr PP, + const CanonicalIncludes &CanonIncludes) { EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; IndexUpdated = true; - Index.updatePreamble(FooCpp, Ctx, std::move(PP)); + Index.updatePreamble(FooCpp, Ctx, std::move(PP), CanonIncludes); }); ASSERT_TRUE(IndexUpdated); @@ -358,7 +363,8 @@ MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI, std::make_shared(), /*StoreInMemory=*/true, - [&](ASTContext &Ctx, std::shared_ptr PP) {}); + [&](ASTContext &Ctx, std::shared_ptr PP, + const CanonicalIncludes &) {}); // Build AST for main file with preamble. auto AST = ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData, Index: unittests/clangd/TestTU.cpp =================================================================== --- unittests/clangd/TestTU.cpp +++ unittests/clangd/TestTU.cpp @@ -59,13 +59,15 @@ SymbolSlab TestTU::headerSymbols() const { auto AST = build(); - return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr()); + return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(), + AST.getCanonicalIncludes()); } std::unique_ptr TestTU::index() const { auto AST = build(); auto Idx = llvm::make_unique(/*UseDex=*/true); - Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr()); + Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(), + AST.getCanonicalIncludes()); Idx->updateMain(Filename, AST); return std::move(Idx); }