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,10 @@ const llvm::StringSet<> &ReferencedPublicHeaders); std::vector computeUnusedIncludes(ParsedAST &AST); +// The same as computeUnusedIncludes, but it is an experimental and +// include-cleaner-lib-based implementation. +std::vector +computeUnusedIncludesExperimental(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-lib-based implementation. std::vector computeUnusedIncludes(ParsedAST &AST) { const auto &SM = AST.getSourceManager(); @@ -469,11 +474,62 @@ translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM); return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas); } +std::vector computeUnusedIncludesExperimental(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 +543,11 @@ .getFileEntryRefForID(AST.getSourceManager().getMainFileID()) ->getName() .str(); - for (const auto *Inc : computeUnusedIncludes(AST)) { + const auto &UnusedIncludes = + Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment + ? computeUnusedIncludesExperimental(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,8 @@ /// (!) does not have tokens from the preamble. const syntax::TokenBuffer &getTokens() const { return Tokens; } + const include_cleaner::PragmaIncludes *getPragmaIncludes() const; + /// Returns the version of the ParseInputs this AST was built from. llvm::StringRef version() const { return Version; } 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" @@ -801,6 +802,12 @@ assert(this->Action); } +const include_cleaner::PragmaIncludes *ParsedAST::getPragmaIncludes() const { + if (!Preamble) + return nullptr; + return &Preamble->Pragmas; +} + std::optional ParsedAST::preambleVersion() const { if (!Preamble) return std::nullopt; 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; + // Captures #include-mapping information in #included headers. + 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 @@ -342,6 +342,8 @@ auto AST = TU.build(); EXPECT_THAT(computeUnusedIncludes(AST), ElementsAre(Pointee(writtenInclusion("")))); + EXPECT_THAT(computeUnusedIncludesExperimental(AST), + ElementsAre(Pointee(writtenInclusion("")))); } TEST(IncludeCleaner, GetUnusedHeaders) { @@ -377,6 +379,10 @@ computeUnusedIncludes(AST), UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")), Pointee(writtenInclusion("\"dir/unused.h\"")))); + EXPECT_THAT( + computeUnusedIncludesExperimental(AST), + UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")), + Pointee(writtenInclusion("\"dir/unused.h\"")))); } TEST(IncludeCleaner, VirtualBuffers) { @@ -531,6 +537,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( @@ -545,6 +554,7 @@ ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID())); EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); + EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); } TEST(IncludeCleaner, RecursiveInclusion) { @@ -573,6 +583,7 @@ EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty())); EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty()); + EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); } TEST(IncludeCleaner, IWYUPragmaExport) { @@ -597,6 +608,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(computeUnusedIncludesExperimental(AST), IsEmpty()); } TEST(IncludeCleaner, NoDiagsForObjC) {