diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 82323fe16c82b..06b949bc4a2b5 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -1194,7 +1194,7 @@ void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo &HI, const SourceManager &SM = AST.getSourceManager(); llvm::SmallVector RankedProviders = - include_cleaner::headersForSymbol(Sym, SM, AST.getPragmaIncludes().get()); + include_cleaner::headersForSymbol(Sym, SM, &AST.getPragmaIncludes()); if (RankedProviders.empty()) return; @@ -1254,7 +1254,7 @@ void maybeAddUsedSymbols(ParsedAST &AST, HoverInfo &HI, const Inclusion &Inc) { llvm::DenseSet UsedSymbols; include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), collectMacroReferences(AST), - AST.getPragmaIncludes().get(), AST.getPreprocessor(), + &AST.getPragmaIncludes(), AST.getPreprocessor(), [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { if (Ref.RT != include_cleaner::RefType::Explicit || diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 2f34c94934920..563a1f5d22f5b 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -311,7 +311,7 @@ getUnused(ParsedAST &AST, auto IncludeID = static_cast(*MFI.HeaderID); if (ReferencedFiles.contains(IncludeID)) continue; - if (!mayConsiderUnused(MFI, AST, AST.getPragmaIncludes().get())) { + if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes())) { dlog("{0} was not used, but is not eligible to be diagnosed as unused", MFI.Written); continue; @@ -403,7 +403,7 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { .getBuiltinDir(); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, - AST.getPragmaIncludes().get(), AST.getPreprocessor(), + &AST.getPragmaIncludes(), AST.getPreprocessor(), [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { bool Satisfied = false; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index d91ce7283ecee..14a91797f4d2e 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -653,6 +653,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, } IncludeStructure Includes; + include_cleaner::PragmaIncludes PI; // If we are using a preamble, copy existing includes. if (Preamble) { Includes = Preamble->Includes; @@ -660,11 +661,15 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, // Replay the preamble includes so that clang-tidy checks can see them. ReplayPreamble::attach(Patch->preambleIncludes(), *Clang, Patch->modifiedBounds()); + PI = *Preamble->Pragmas; } // Important: collectIncludeStructure is registered *after* ReplayPreamble! // Otherwise we would collect the replayed includes again... // (We can't *just* use the replayed includes, they don't have Resolved path). Includes.collect(*Clang); + // Same for pragma-includes, we're already inheriting preamble includes, so we + // should only receive callbacks for non-preamble mainfile includes. + PI.record(*Clang); // Copy over the macros in the preamble region of the main file, and combine // with non-preamble macros below. MainFileMacros Macros; @@ -735,7 +740,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, ParsedAST Result(Filename, Inputs.Version, std::move(Preamble), std::move(Clang), std::move(Action), std::move(Tokens), std::move(Macros), std::move(Marks), std::move(ParsedDecls), - std::move(Diags), std::move(Includes)); + std::move(Diags), std::move(Includes), std::move(PI)); llvm::move(getIncludeCleanerDiags(Result, Inputs.Contents), std::back_inserter(Result.Diags)); return std::move(Result); @@ -828,23 +833,21 @@ ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector Marks, std::vector LocalTopLevelDecls, - std::vector Diags, IncludeStructure Includes) + std::vector Diags, IncludeStructure Includes, + include_cleaner::PragmaIncludes PI) : TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)), Action(std::move(Action)), Tokens(std::move(Tokens)), Macros(std::move(Macros)), Marks(std::move(Marks)), Diags(std::move(Diags)), LocalTopLevelDecls(std::move(LocalTopLevelDecls)), - Includes(std::move(Includes)) { - Resolver = std::make_unique(getASTContext()); + Includes(std::move(Includes)), PI(std::move(PI)), + Resolver(std::make_unique(getASTContext())) { assert(this->Clang); assert(this->Action); } -std::shared_ptr -ParsedAST::getPragmaIncludes() const { - if (!Preamble) - return nullptr; - return Preamble->Pragmas; +const include_cleaner::PragmaIncludes &ParsedAST::getPragmaIncludes() const { + return PI; } std::optional ParsedAST::preambleVersion() const { diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index c68fdba6bd26c..63e564bd68a78 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -103,10 +103,8 @@ class ParsedAST { /// Tokens recorded while parsing the main file. /// (!) does not have tokens from the preamble. const syntax::TokenBuffer &getTokens() const { return Tokens; } - /// Returns the PramaIncludes from the preamble. - /// Might be null if AST is built without a preamble. - std::shared_ptr - getPragmaIncludes() const; + /// Returns the PramaIncludes for preamble + main file includes. + const include_cleaner::PragmaIncludes &getPragmaIncludes() const; /// Returns the version of the ParseInputs this AST was built from. llvm::StringRef version() const { return Version; } @@ -129,7 +127,7 @@ class ParsedAST { std::unique_ptr Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector Marks, std::vector LocalTopLevelDecls, std::vector Diags, - IncludeStructure Includes); + IncludeStructure Includes, include_cleaner::PragmaIncludes PI); Path TUPath; std::string Version; // In-memory preambles must outlive the AST, it is important that this member @@ -159,6 +157,7 @@ class ParsedAST { // top-level decls from the preamble. std::vector LocalTopLevelDecls; IncludeStructure Includes; + include_cleaner::PragmaIncludes PI; std::unique_ptr Resolver; }; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index f55d2a5623956..5f41f788a6939 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1339,7 +1339,7 @@ maybeFindIncludeReferences(ParsedAST &AST, Position Pos, auto Converted = convertIncludes(AST); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), collectMacroReferences(AST), - AST.getPragmaIncludes().get(), AST.getPreprocessor(), + &AST.getPragmaIncludes(), AST.getPreprocessor(), [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef Providers) { if (Ref.RT != include_cleaner::RefType::Explicit || diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp index 5052c661cfc5e..eb9562d2b6bf8 100644 --- a/clang-tools-extra/clangd/index/FileIndex.cpp +++ b/clang-tools-extra/clangd/index/FileIndex.cpp @@ -223,7 +223,7 @@ FileShardedIndex::getShard(llvm::StringRef Uri) const { SlabTuple indexMainDecls(ParsedAST &AST) { return indexSymbols( AST.getASTContext(), AST.getPreprocessor(), AST.getLocalTopLevelDecls(), - &AST.getMacros(), *AST.getPragmaIncludes(), + &AST.getMacros(), AST.getPragmaIncludes(), /*IsIndexMainAST=*/true, AST.version(), /*CollectMainFileRefs=*/true); } diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp index cf30b388d234d..9f713564b2c01 100644 --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -176,7 +176,7 @@ void update(FileIndex &M, llvm::StringRef Basename, llvm::StringRef Code) { auto AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); } TEST(FileIndexTest, CustomizedURIScheme) { @@ -254,7 +254,7 @@ TEST(FileIndexTest, IWYUPragmaExport) { auto AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); auto Symbols = runFuzzyFind(M, ""); EXPECT_THAT( @@ -446,7 +446,7 @@ TEST(FileIndexTest, Relations) { FileIndex Index; Index.updatePreamble(testPath(TU.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); SymbolID A = findSymbol(TU.headerSymbols(), "A").ID; uint32_t Results = 0; RelationsRequest Req; @@ -567,7 +567,7 @@ TEST(FileIndexTest, StalePreambleSymbolsDeleted) { auto AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); EXPECT_THAT(runFuzzyFind(M, ""), UnorderedElementsAre(qName("a"))); File.Filename = "f2.cpp"; @@ -575,7 +575,7 @@ TEST(FileIndexTest, StalePreambleSymbolsDeleted) { AST = File.build(); M.updatePreamble(testPath(File.Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); EXPECT_THAT(runFuzzyFind(M, ""), UnorderedElementsAre(qName("b"))); } @@ -720,7 +720,7 @@ TEST(FileIndexTest, Profile) { auto AST = TestTU::withHeaderCode("int a;").build(); FI.updateMain(FileName, AST); FI.updatePreamble(FileName, "v1", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); llvm::BumpPtrAllocator Alloc; MemoryTree MT(&Alloc); diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 5a6524dec2f09..7ed4a9103e1f2 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -316,8 +316,10 @@ TEST(IncludeCleaner, IWYUPragmas) { #include "public.h" void bar() { foo(); } + #include "keep_main_file.h" // IWYU pragma: keep )cpp"; TU.AdditionalFiles["behind_keep.h"] = guard(""); + TU.AdditionalFiles["keep_main_file.h"] = guard(""); TU.AdditionalFiles["exported.h"] = guard(""); TU.AdditionalFiles["public.h"] = guard("#include \"private.h\""); TU.AdditionalFiles["private.h"] = guard(R"cpp( diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index e65ae825b4167..1f02c04125b1e 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -164,7 +164,7 @@ SymbolSlab TestTU::headerSymbols() const { auto AST = build(); return std::get<0>(indexHeaderSymbols( /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes())); + AST.getPragmaIncludes())); } RefSlab TestTU::headerRefs() const { @@ -177,7 +177,7 @@ std::unique_ptr TestTU::index() const { auto Idx = std::make_unique(); Idx->updatePreamble(testPath(Filename), /*Version=*/"null", AST.getASTContext(), AST.getPreprocessor(), - *AST.getPragmaIncludes()); + AST.getPragmaIncludes()); Idx->updateMain(testPath(Filename), AST); return std::move(Idx); } diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h index 2e0b462ce16df..2dcb5ea2555c5 100644 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h @@ -113,7 +113,8 @@ class PragmaIncludes { llvm::DenseSet ShouldKeep; /// Owns the strings. - llvm::BumpPtrAllocator Arena; + /// Each record() pushes a new one, while keeping all the old strings alive. + std::vector> Arena; // FIXME: add support for clang use_instead pragma }; diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp index bd726cff12a97..c93c56adf650d 100644 --- a/clang-tools-extra/include-cleaner/lib/Record.cpp +++ b/clang-tools-extra/include-cleaner/lib/Record.cpp @@ -178,7 +178,8 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler { : RecordPragma(CI.getPreprocessor(), Out) {} RecordPragma(const Preprocessor &P, PragmaIncludes *Out) : SM(P.getSourceManager()), HeaderInfo(P.getHeaderSearchInfo()), Out(Out), - UniqueStrings(Arena) {} + Arena(std::make_shared()), + UniqueStrings(*Arena) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -204,7 +205,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler { std::unique(It.getSecond().begin(), It.getSecond().end()), It.getSecond().end()); } - Out->Arena = std::move(Arena); + Out->Arena.emplace_back(std::move(Arena)); } void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, @@ -336,7 +337,7 @@ class PragmaIncludes::RecordPragma : public PPCallbacks, public CommentHandler { const SourceManager &SM; const HeaderSearch &HeaderInfo; PragmaIncludes *Out; - llvm::BumpPtrAllocator Arena; + std::shared_ptr Arena; /// Intern table for strings. Contents are on the arena. llvm::StringSaver UniqueStrings; diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp index 0f2ded5f18345..d1f7590b22268 100644 --- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp @@ -588,5 +588,30 @@ TEST_F(PragmaIncludeTest, OutlivesFMAndSM) { EXPECT_THAT(PI.getExporters(Private2FE.get(), FM), testing::ElementsAre(llvm::cantFail(FM.getFileRef("public.h")))); } + +TEST_F(PragmaIncludeTest, CanRecordManyTimes) { + Inputs.Code = R"cpp( + #include "public.h" + )cpp"; + Inputs.ExtraFiles["public.h"] = R"cpp( + #include "private.h" + )cpp"; + Inputs.ExtraFiles["private.h"] = R"cpp( + // IWYU pragma: private, include "public.h" + )cpp"; + + TestAST Processed = build(); + auto &FM = Processed.fileManager(); + auto PrivateFE = FM.getFile("private.h"); + llvm::StringRef Public = PI.getPublic(PrivateFE.get()); + EXPECT_EQ(Public, "\"public.h\""); + + // This build populates same PI during build, but this time we don't have + // any IWYU pragmas. Make sure strings from previous recordings are still + // alive. + Inputs.Code = ""; + build(); + EXPECT_EQ(Public, "\"public.h\""); +} } // namespace } // namespace clang::include_cleaner