https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/75612
This makes PragmaIncldues copyable, and copies it from preamble when building a new AST. Fixes https://github.com/clangd/clangd/issues/1843 Fixes https://github.com/clangd/clangd/issues/1571 From 8801af07ad4e469f832570b5027e1522f41a6d06 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Fri, 15 Dec 2023 15:14:05 +0100 Subject: [PATCH] [clangd] Track IWYU pragmas for non-preamble includes --- clang-tools-extra/clangd/Hover.cpp | 4 ++-- clang-tools-extra/clangd/IncludeCleaner.cpp | 4 ++-- clang-tools-extra/clangd/ParsedAST.cpp | 21 +++++++++++-------- clang-tools-extra/clangd/ParsedAST.h | 9 ++++---- clang-tools-extra/clangd/XRefs.cpp | 2 +- clang-tools-extra/clangd/index/FileIndex.cpp | 2 +- .../clangd/unittests/FileIndexTests.cpp | 12 +++++------ .../clangd/unittests/IncludeCleanerTests.cpp | 2 ++ clang-tools-extra/clangd/unittests/TestTU.cpp | 4 ++-- .../include/clang-include-cleaner/Record.h | 2 +- .../include-cleaner/lib/Record.cpp | 5 +++-- 11 files changed, 36 insertions(+), 31 deletions(-) diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index 82323fe16c82b6..06b949bc4a2b55 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<include_cleaner::Header> 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<include_cleaner::Symbol> UsedSymbols; include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), collectMacroReferences(AST), - AST.getPragmaIncludes().get(), AST.getPreprocessor(), + &AST.getPragmaIncludes(), AST.getPreprocessor(), [&](const include_cleaner::SymbolReference &Ref, llvm::ArrayRef<include_cleaner::Header> 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 2f34c949349200..563a1f5d22f5b5 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<IncludeStructure::HeaderID>(*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<include_cleaner::Header> Providers) { bool Satisfied = false; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index d91ce7283ecee4..14a91797f4d2ea 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<PragmaMark> Marks, std::vector<Decl *> LocalTopLevelDecls, - std::vector<Diag> Diags, IncludeStructure Includes) + std::vector<Diag> 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<HeuristicResolver>(getASTContext()); + Includes(std::move(Includes)), PI(std::move(PI)), + Resolver(std::make_unique<HeuristicResolver>(getASTContext())) { assert(this->Clang); assert(this->Action); } -std::shared_ptr<const include_cleaner::PragmaIncludes> -ParsedAST::getPragmaIncludes() const { - if (!Preamble) - return nullptr; - return Preamble->Pragmas; +const include_cleaner::PragmaIncludes &ParsedAST::getPragmaIncludes() const { + return PI; } std::optional<llvm::StringRef> ParsedAST::preambleVersion() const { diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index c68fdba6bd26cd..63e564bd68a78c 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<const include_cleaner::PragmaIncludes> - 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<FrontendAction> Action, syntax::TokenBuffer Tokens, MainFileMacros Macros, std::vector<PragmaMark> Marks, std::vector<Decl *> LocalTopLevelDecls, std::vector<Diag> 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<Decl *> LocalTopLevelDecls; IncludeStructure Includes; + include_cleaner::PragmaIncludes PI; std::unique_ptr<HeuristicResolver> Resolver; }; diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index f55d2a56239563..5f41f788a69393 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<include_cleaner::Header> 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 5052c661cfc5ef..eb9562d2b6bf81 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 cf30b388d234df..9f713564b2c01f 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 5a6524dec2f09a..7ed4a9103e1f24 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 e65ae825b41677..1f02c04125b1ea 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<SymbolIndex> TestTU::index() const { auto Idx = std::make_unique<FileIndex>(); 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 2e0b462ce16df1..5e245736d4dbb4 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,7 @@ class PragmaIncludes { llvm::DenseSet<llvm::sys::fs::UniqueID> ShouldKeep; /// Owns the strings. - llvm::BumpPtrAllocator Arena; + std::shared_ptr<llvm::BumpPtrAllocator> 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 bd726cff12a97d..a606b109e5c2e9 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<llvm::BumpPtrAllocator>()), + UniqueStrings(*Arena) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -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<llvm::BumpPtrAllocator> Arena; /// Intern table for strings. Contents are on the arena. llvm::StringSaver UniqueStrings; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
