diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -59,6 +59,7 @@ endif() include_directories(BEFORE "${CMAKE_CURRENT_BINARY_DIR}/../clang-tidy") +include_directories(BEFORE "${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include") add_clang_library(clangDaemon AST.cpp @@ -162,6 +163,7 @@ clangDriver clangFormat clangFrontend + clangIncludeCleaner clangIndex clangLex clangSema diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -88,7 +88,13 @@ bool StandardLibrary = true; } Index; - enum UnusedIncludesPolicy { Strict, None }; + enum UnusedIncludesPolicy { + /// Diagnose unused includes. + Strict, + None, + /// The same as Strict, but using the include-cleaner library. + Experiment, + }; /// Controls warnings and errors when parsing code. struct { bool SuppressAll = false; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -431,11 +431,13 @@ }); if (F.UnusedIncludes) - if (auto Val = compileEnum( - "UnusedIncludes", **F.UnusedIncludes) - .map("Strict", Config::UnusedIncludesPolicy::Strict) - .map("None", Config::UnusedIncludesPolicy::None) - .value()) + if (auto Val = + compileEnum("UnusedIncludes", + **F.UnusedIncludes) + .map("Strict", Config::UnusedIncludesPolicy::Strict) + .map("Experiment", Config::UnusedIncludesPolicy::Experiment) + .map("None", Config::UnusedIncludesPolicy::None) + .value()) Out.Apply.push_back([Val](const Params &, Config &C) { C.Diagnostics.UnusedIncludes = *Val; }); diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -97,6 +97,7 @@ const llvm::StringSet<> &ReferencedPublicHeaders); std::vector computeUnusedIncludes(ParsedAST &AST); +std::vector computeUnusedIncludesNew(ParsedAST &AST); std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, llvm::StringRef Code); diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -12,6 +12,8 @@ #include "ParsedAST.h" #include "Protocol.h" #include "SourceCode.h" +#include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Types.h" #include "index/CanonicalIncludes.h" #include "support/Logger.h" #include "support/Trace.h" @@ -458,6 +460,9 @@ return TranslatedHeaderIDs; } +// This is the original clangd-own implementation for computing unused +// #includes. Eventually it will be deprecated and replaced by the +// include-cleaner-library-based implementation. std::vector computeUnusedIncludes(ParsedAST &AST) { const auto &SM = AST.getSourceManager(); @@ -469,11 +474,63 @@ translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); } +// An include-cleaner-based implementation of computeUnusedIncludes. +std::vector computeUnusedIncludesNew(ParsedAST &AST) { + const auto &SM = AST.getSourceManager(); + const auto &Includes = AST.getIncludeStructure(); + // FIXME: this map should probably be in IncludeStructure. + llvm::StringMap> BySpelling; + for (const auto &Inc : Includes.MainFileIncludes) { + if (Inc.HeaderID) + BySpelling.try_emplace(Inc.Written) + .first->second.push_back( + static_cast(*Inc.HeaderID)); + } + // FIXME: !!this is a hacky way to collect macro references. + std::vector Macros; + auto& PP = AST.getPreprocessor(); + for (const syntax::Token &Tok : + AST.getTokens().spelledTokens(SM.getMainFileID())) { + auto Macro = locateMacroAt(Tok, PP); + if (!Macro) + continue; + if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid()) + Macros.push_back( + {Tok.location(), + include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)), + DefLoc}, + include_cleaner::RefType::Explicit}); + } + llvm::DenseSet Used; + include_cleaner::walkUsed( + AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, + &AST.getPragmaIncludes(), SM, + [&](const include_cleaner::SymbolReference &Ref, + llvm::ArrayRef Providers) { + for (const auto &H : Providers) { + switch (H.kind()) { + case include_cleaner::Header::Physical: + if (auto HeaderID = Includes.getID(H.physical())) + Used.insert(*HeaderID); + break; + case include_cleaner::Header::Standard: + for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard())) + Used.insert(HeaderID); + break; + case include_cleaner::Header::Verbatim: + for (auto HeaderID : BySpelling.lookup(H.verbatim())) + Used.insert(HeaderID); + break; + } + } + }); + return getUnused(AST, Used, /*ReferencedPublicHeaders*/{}); +} std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, llvm::StringRef Code) { const Config &Cfg = Config::current(); - if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict || + if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None || Cfg.Diagnostics.SuppressAll || Cfg.Diagnostics.Suppress.contains("unused-includes")) return {}; @@ -487,7 +544,11 @@ .getFileEntryRefForID(AST.getSourceManager().getMainFileID()) ->getName() .str(); - for (const auto *Inc : computeUnusedIncludes(AST)) { + const auto &UnusedIncludes = + Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment + ? computeUnusedIncludesNew(AST) + : computeUnusedIncludes(AST); + for (const auto *Inc : UnusedIncludes) { Diag D; D.Message = llvm::formatv("included header {0} is not used directly", diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -25,6 +25,7 @@ #include "Diagnostics.h" #include "Headers.h" #include "Preamble.h" +#include "clang-include-cleaner/Record.h" #include "index/CanonicalIncludes.h" #include "support/Path.h" #include "clang/Frontend/FrontendAction.h" @@ -107,6 +108,10 @@ /// (!) does not have tokens from the preamble. const syntax::TokenBuffer &getTokens() const { return Tokens; } + const include_cleaner::PragmaIncludes &getPragmaIncludes() const { + return Pragmas; + } + /// Returns the version of the ParseInputs this AST was built from. llvm::StringRef version() const { return Version; } @@ -129,7 +134,8 @@ MainFileMacros Macros, std::vector Marks, std::vector LocalTopLevelDecls, std::optional> Diags, IncludeStructure Includes, - CanonicalIncludes CanonIncludes); + CanonicalIncludes CanonIncludes, + include_cleaner::PragmaIncludes Pragmas); Path TUPath; std::string Version; @@ -161,6 +167,7 @@ std::vector LocalTopLevelDecls; IncludeStructure Includes; CanonicalIncludes CanonIncludes; + include_cleaner::PragmaIncludes Pragmas; std::unique_ptr Resolver; }; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -23,6 +23,7 @@ #include "Preamble.h" #include "SourceCode.h" #include "TidyProvider.h" +#include "clang-include-cleaner/Record.h" #include "index/CanonicalIncludes.h" #include "index/Index.h" #include "index/Symbol.h" @@ -589,9 +590,11 @@ } IncludeStructure Includes; + include_cleaner::PragmaIncludes Pragmas; // If we are using a preamble, copy existing includes. if (Preamble) { Includes = Preamble->Includes; + Pragmas = Preamble->Pragmas; Includes.MainFileIncludes = Patch->preambleIncludes(); // Replay the preamble includes so that clang-tidy checks can see them. ReplayPreamble::attach(Patch->preambleIncludes(), *Clang, @@ -601,6 +604,9 @@ // 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); + if (Config::current().Diagnostics.UnusedIncludes == + Config::UnusedIncludesPolicy::Experiment) + Pragmas.record(*Clang); // Copy over the macros in the preamble region of the main file, and combine // with non-preamble macros below. MainFileMacros Macros; @@ -686,7 +692,7 @@ 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(CanonIncludes)); + std::move(CanonIncludes), std::move(Pragmas)); if (Result.Diags) { auto UnusedHeadersDiags = issueUnusedIncludesDiagnostics(Result, Inputs.Contents); @@ -789,13 +795,15 @@ std::vector Marks, std::vector LocalTopLevelDecls, std::optional> Diags, - IncludeStructure Includes, CanonicalIncludes CanonIncludes) + IncludeStructure Includes, CanonicalIncludes CanonIncludes, + include_cleaner::PragmaIncludes Pragmas) : 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)), CanonIncludes(std::move(CanonIncludes)) { + Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)), + Pragmas(std::move(Pragmas)) { Resolver = std::make_unique(getASTContext()); assert(this->Clang); assert(this->Action); diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -27,6 +27,7 @@ #include "Diagnostics.h" #include "FS.h" #include "Headers.h" +#include "clang-include-cleaner/Record.h" #include "index/CanonicalIncludes.h" #include "support/Path.h" #include "clang/Frontend/CompilerInvocation.h" @@ -57,6 +58,8 @@ // Processes like code completions and go-to-definitions will need #include // information, and their compile action skips preamble range. IncludeStructure Includes; + + include_cleaner::PragmaIncludes Pragmas; // Macros defined in the preamble section of the main file. // Users care about headers vs main-file, not preamble vs non-preamble. // These should be treated as main-file entities e.g. for code completion. diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -11,6 +11,7 @@ #include "Config.h" #include "Headers.h" #include "SourceCode.h" +#include "clang-include-cleaner/Record.h" #include "support/Logger.h" #include "support/ThreadsafeFS.h" #include "support/Trace.h" @@ -77,6 +78,9 @@ std::vector takeMarks() { return std::move(Marks); } + include_cleaner::PragmaIncludes takePragmaIncludes() { + return std::move(Pragmas); + } CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; } @@ -118,6 +122,9 @@ LangOpts = &CI.getLangOpts(); SourceMgr = &CI.getSourceManager(); Includes.collect(CI); + if (Config::current().Diagnostics.UnusedIncludes == + Config::UnusedIncludesPolicy::Experiment) + Pragmas.record(CI); if (BeforeExecuteCallback) BeforeExecuteCallback(CI); } @@ -187,6 +194,7 @@ PreambleParsedCallback ParsedCallback; IncludeStructure Includes; CanonicalIncludes CanonIncludes; + include_cleaner::PragmaIncludes Pragmas; MainFileMacros Macros; std::vector Marks; bool IsMainFileIncludeGuarded = false; @@ -560,6 +568,7 @@ Result->CompileCommand = Inputs.CompileCommand; Result->Diags = std::move(Diags); Result->Includes = CapturedInfo.takeIncludes(); + Result->Pragmas = CapturedInfo.takePragmaIncludes(); Result->Macros = CapturedInfo.takeMacros(); Result->Marks = CapturedInfo.takeMarks(); Result->CanonIncludes = CapturedInfo.takeCanonicalIncludes(); diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -343,6 +343,8 @@ auto Unused = computeUnusedIncludes(AST); EXPECT_THAT(Unused, ElementsAre(Pointee(writtenInclusion("")))); + EXPECT_THAT(computeUnusedIncludesNew(AST), + ElementsAre(Pointee(writtenInclusion("")))); } TEST(IncludeCleaner, GetUnusedHeaders) { @@ -379,6 +381,10 @@ UnusedIncludes.push_back(Include->Written); EXPECT_THAT(UnusedIncludes, UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\"")); + EXPECT_THAT( + computeUnusedIncludesNew(AST), + UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")), + Pointee(writtenInclusion("\"dir/unused.h\"")))); } TEST(IncludeCleaner, VirtualBuffers) { @@ -533,6 +539,9 @@ // IWYU pragma: private, include "public.h" void foo() {} )cpp"); + Config Cfg; + Cfg.Diagnostics.UnusedIncludes = Config::Experiment; + WithContextValue Ctx(Config::Key, std::move(Cfg)); ParsedAST AST = TU.build(); auto ReferencedFiles = findReferencedFiles( @@ -547,6 +556,7 @@ ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID())); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); + EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty()); } TEST(IncludeCleaner, RecursiveInclusion) { @@ -575,6 +585,7 @@ EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); + EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty()); } TEST(IncludeCleaner, IWYUPragmaExport) { @@ -599,6 +610,7 @@ // FIXME: This is not correct: foo.h is unused but is not diagnosed as such // because we ignore headers with IWYU export pragmas for now. EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); + EXPECT_THAT(computeUnusedIncludesNew(AST), IsEmpty()); } TEST(IncludeCleaner, NoDiagsForObjC) { 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 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h @@ -48,6 +48,13 @@ /// defines the symbol. class PragmaIncludes { public: + PragmaIncludes() = default; + PragmaIncludes(PragmaIncludes &&) = default; + PragmaIncludes &operator=(PragmaIncludes &&) = default; + + PragmaIncludes(const PragmaIncludes &); + PragmaIncludes &operator=(const PragmaIncludes &); + /// Installs an analysing PPCallback and CommentHandler and populates results /// to the structure. void record(const CompilerInstance &CI); diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp --- a/clang-tools-extra/include-cleaner/lib/Record.cpp +++ b/clang-tools-extra/include-cleaner/lib/Record.cpp @@ -150,7 +150,7 @@ RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out) : SM(CI.getSourceManager()), HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out), - UniqueStrings(Arena) {} + UniqueStrings(Out->Arena) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -176,7 +176,6 @@ std::unique(It.getSecond().begin(), It.getSecond().end()), It.getSecond().end()); } - Out->Arena = std::move(Arena); } void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, @@ -307,7 +306,6 @@ const SourceManager &SM; HeaderSearch &HeaderInfo; PragmaIncludes *Out; - llvm::BumpPtrAllocator Arena; /// Intern table for strings. Contents are on the arena. llvm::StringSaver UniqueStrings; @@ -336,6 +334,27 @@ std::vector KeepStack; }; +PragmaIncludes::PragmaIncludes(const PragmaIncludes & In) { + *this = In; +} + +PragmaIncludes &PragmaIncludes::operator=(const PragmaIncludes &In) { + ShouldKeep = In.ShouldKeep; + NonSelfContainedFiles = In.NonSelfContainedFiles; + Arena.Reset(); + llvm::UniqueStringSaver Strings(Arena); + IWYUPublic.clear(); + for (const auto& It : In.IWYUPublic) + IWYUPublic.try_emplace(It.first, Strings.save(It.second)); + IWYUExportBy.clear(); + for (const auto& UIDAndExporters : In.IWYUExportBy) { + const auto& [UID, Exporters] = UIDAndExporters; + for (StringRef ExporterHeader : Exporters) + IWYUExportBy.try_emplace(UID).first->second.push_back(ExporterHeader); + } + return *this; + } + void PragmaIncludes::record(const CompilerInstance &CI) { auto Record = std::make_unique(CI, this); CI.getPreprocessor().addCommentHandler(Record.get());