https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/87208
>From 35db92dfd0c2b43fc7afde5b093598763c4b8c89 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin <vvd170...@gmail.com> Date: Mon, 1 Apr 2024 02:26:14 +0300 Subject: [PATCH 1/4] Add config option to analyze unused system headers --- clang-tools-extra/clangd/Config.h | 1 + clang-tools-extra/clangd/ConfigCompile.cpp | 57 +++++++++++-------- clang-tools-extra/clangd/ConfigFragment.h | 4 ++ clang-tools-extra/clangd/ConfigYAML.cpp | 4 ++ clang-tools-extra/clangd/IncludeCleaner.cpp | 34 +++++++---- clang-tools-extra/clangd/IncludeCleaner.h | 13 +---- clang-tools-extra/clangd/ParsedAST.cpp | 3 +- .../clangd/unittests/ConfigCompileTests.cpp | 6 ++ .../clangd/unittests/ConfigYAMLTests.cpp | 15 +++++ .../clangd/unittests/IncludeCleanerTests.cpp | 15 +++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ 11 files changed, 112 insertions(+), 45 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 4371c80a6c587..9629854abff31 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -114,6 +114,7 @@ struct Config { /// these regexes. struct { std::vector<std::function<bool(llvm::StringRef)>> IgnoreHeader; + bool AnalyzeSystemHeaders = false; } Includes; } Diagnostics; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index 5bb2eb4a9f803..f74c870adfb73 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -572,32 +572,43 @@ struct FragmentCompiler { #else static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags; #endif - auto Filters = std::make_shared<std::vector<llvm::Regex>>(); - for (auto &HeaderPattern : F.IgnoreHeader) { - // Anchor on the right. - std::string AnchoredPattern = "(" + *HeaderPattern + ")$"; - llvm::Regex CompiledRegex(AnchoredPattern, Flags); - std::string RegexError; - if (!CompiledRegex.isValid(RegexError)) { - diag(Warning, - llvm::formatv("Invalid regular expression '{0}': {1}", - *HeaderPattern, RegexError) - .str(), - HeaderPattern.Range); - continue; + std::shared_ptr<std::vector<llvm::Regex>> Filters; + if (!F.IgnoreHeader.empty()) { + Filters = std::make_shared<std::vector<llvm::Regex>>(); + for (auto &HeaderPattern : F.IgnoreHeader) { + // Anchor on the right. + std::string AnchoredPattern = "(" + *HeaderPattern + ")$"; + llvm::Regex CompiledRegex(AnchoredPattern, Flags); + std::string RegexError; + if (!CompiledRegex.isValid(RegexError)) { + diag(Warning, + llvm::formatv("Invalid regular expression '{0}': {1}", + *HeaderPattern, RegexError) + .str(), + HeaderPattern.Range); + continue; + } + Filters->push_back(std::move(CompiledRegex)); } - Filters->push_back(std::move(CompiledRegex)); } - if (Filters->empty()) + std::optional<bool> AnalyzeSystemHeaders; + if (F.AnalyzeSystemHeaders.has_value()) + AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders; + if (!Filters && !AnalyzeSystemHeaders.has_value()) return; - auto Filter = [Filters](llvm::StringRef Path) { - for (auto &Regex : *Filters) - if (Regex.match(Path)) - return true; - return false; - }; - Out.Apply.push_back([Filter](const Params &, Config &C) { - C.Diagnostics.Includes.IgnoreHeader.emplace_back(Filter); + Out.Apply.push_back([Filters = std::move(Filters), + AnalyzeSystemHeaders](const Params &, Config &C) { + if (Filters) { + auto Filter = [Filters](llvm::StringRef Path) { + for (auto &Regex : *Filters) + if (Regex.match(Path)) + return true; + return false; + }; + C.Diagnostics.Includes.IgnoreHeader.emplace_back(std::move(Filter)); + } + if (AnalyzeSystemHeaders.has_value()) + C.Diagnostics.Includes.AnalyzeSystemHeaders = *AnalyzeSystemHeaders; }); } diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 7fa61108c78a0..ac8b88c245412 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -254,6 +254,10 @@ struct Fragment { /// unused or missing. These can match any suffix of the header file in /// question. std::vector<Located<std::string>> IgnoreHeader; + + /// If false (default), unused system headers will be ignored. + /// Standard library headers are analyzed regardless of this option. + std::optional<Located<bool>> AnalyzeSystemHeaders; }; IncludesBlock Includes; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index ce09af819247a..7608af4200538 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -169,6 +169,10 @@ class Parser { if (auto Values = scalarValues(N)) F.IgnoreHeader = std::move(*Values); }); + Dict.handle("AnalyzeSystemHeaders", [&](Node &N) { + if (auto Value = boolValue(N, "AnalyzeSystemHeaders")) + F.AnalyzeSystemHeaders = *Value; + }); Dict.parse(N); } diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 8e48f546d94e7..0fdd0412d1e48 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -68,24 +68,32 @@ bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) { } bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, - const include_cleaner::PragmaIncludes *PI) { + const include_cleaner::PragmaIncludes *PI, + bool AnalyzeSystemHeaders) { assert(Inc.HeaderID); auto HID = static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID); auto FE = AST.getSourceManager().getFileManager().getFileRef( AST.getIncludeStructure().getRealPath(HID)); assert(FE); if (FE->getDir() == AST.getPreprocessor() - .getHeaderSearchInfo() - .getModuleMap() - .getBuiltinDir()) + .getHeaderSearchInfo() + .getModuleMap() + .getBuiltinDir()) return false; if (PI && PI->shouldKeep(*FE)) return false; // FIXME(kirillbobyrev): We currently do not support the umbrella headers. // System headers are likely to be standard library headers. - // Until we have good support for umbrella headers, don't warn about them. - if (Inc.Written.front() == '<') - return tooling::stdlib::Header::named(Inc.Written).has_value(); + // Until we have good support for umbrella headers, don't warn about them + // (unless analysis is explicitly enabled). + if (Inc.Written.front() == '<') { + if (tooling::stdlib::Header::named(Inc.Written)) { + return true; + } + if (!AnalyzeSystemHeaders) { + return false; + } + } if (PI) { // Check if main file is the public interface for a private header. If so we // shouldn't diagnose it as unused. @@ -266,7 +274,8 @@ Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) { std::vector<const Inclusion *> getUnused(ParsedAST &AST, - const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles) { + const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles, + bool AnalyzeSystemHeaders) { trace::Span Tracer("IncludeCleaner::getUnused"); std::vector<const Inclusion *> Unused; for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) { @@ -275,7 +284,8 @@ getUnused(ParsedAST &AST, auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID); if (ReferencedFiles.contains(IncludeID)) continue; - if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes())) { + if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes(), + AnalyzeSystemHeaders)) { dlog("{0} was not used, but is not eligible to be diagnosed as unused", MFI.Written); continue; @@ -347,7 +357,8 @@ include_cleaner::Includes convertIncludes(const ParsedAST &AST) { return ConvertedIncludes; } -IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { +IncludeCleanerFindings +computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeSystemHeaders) { // Interaction is only polished for C/CPP. if (AST.getLangOpts().ObjC) return {}; @@ -432,7 +443,8 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { MapInfo::getHashValue(RHS.Symbol); }); MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end()); - std::vector<const Inclusion *> UnusedIncludes = getUnused(AST, Used); + std::vector<const Inclusion *> UnusedIncludes = + getUnused(AST, Used, AnalyzeSystemHeaders); return {std::move(UnusedIncludes), std::move(MissingIncludes)}; } diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index 387763de34076..d03d74038b483 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -53,7 +53,9 @@ struct IncludeCleanerFindings { std::vector<MissingIncludeDiagInfo> MissingIncludes; }; -IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST); +IncludeCleanerFindings +computeIncludeCleanerFindings(ParsedAST &AST, + bool AnalyzeSystemHeaders = false); using HeaderFilter = llvm::ArrayRef<std::function<bool(llvm::StringRef)>>; std::vector<Diag> @@ -62,15 +64,6 @@ issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code, const ThreadsafeFS &TFS, HeaderFilter IgnoreHeader = {}); -/// Affects whether standard library includes should be considered for -/// removal. This is off by default for now due to implementation limitations: -/// - macros are not tracked -/// - symbol names without a unique associated header are not tracked -/// - references to std-namespaced C types are not properly tracked: -/// instead of std::size_t -> <cstddef> we see ::size_t -> <stddef.h> -/// FIXME: remove this hack once the implementation is good enough. -void setIncludeCleanerAnalyzesStdlib(bool B); - /// Converts the clangd include representation to include-cleaner /// include representation. include_cleaner::Includes convertIncludes(const ParsedAST &); diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 3ff759415f7c8..8073b1ee76e64 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -373,7 +373,8 @@ std::vector<Diag> getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code, Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::None; if (SuppressMissing && SuppressUnused) return {}; - auto Findings = computeIncludeCleanerFindings(AST); + auto Findings = computeIncludeCleanerFindings( + AST, Cfg.Diagnostics.Includes.AnalyzeSystemHeaders); if (SuppressMissing) Findings.MissingIncludes.clear(); if (SuppressUnused) diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index f0ffc429c0ca9..20d873525fd16 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -277,6 +277,12 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) { }; EXPECT_TRUE(HeaderFilter("foo.h")); EXPECT_FALSE(HeaderFilter("bar.h")); + + Frag = {}; + EXPECT_FALSE(Conf.Diagnostics.Includes.AnalyzeSystemHeaders); + Frag.Diagnostics.Includes.AnalyzeSystemHeaders = true; + EXPECT_TRUE(compileAndApply()); + EXPECT_TRUE(Conf.Diagnostics.Includes.AnalyzeSystemHeaders); } TEST_F(ConfigCompileTests, DiagnosticSuppression) { diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp index 44a6647d4c0a8..162db1fa34125 100644 --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -278,6 +278,21 @@ TEST(ParseYAML, IncludesIgnoreHeader) { ElementsAre(val("foo"), val("bar"))); } +TEST(ParseYAML, IncludesAnalyzeSystemHeaders) { + CapturedDiags Diags; + Annotations YAML(R"yaml( +Diagnostics: + Includes: + AnalyzeSystemHeaders: true + )yaml"); + auto Results = + Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + ASSERT_EQ(Results.size(), 1u); + EXPECT_THAT(Results[0].Diagnostics.Includes.AnalyzeSystemHeaders, + llvm::ValueIs(val(true))); +} + TEST(ParseYAML, Style) { CapturedDiags Diags; Annotations YAML(R"yaml( diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 142310837bd9c..104c42779383e 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -135,6 +135,21 @@ TEST(IncludeCleaner, GetUnusedHeaders) { Pointee(writtenInclusion("\"dir/unused.h\"")))); } +TEST(IncludeCleaner, SystemUnusedHeaders) { + auto TU = TestTU::withCode(R"cpp( + #include <system_header.h> + #include <system_unused.h> + SystemClass x; + )cpp"); + TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass {};"); + TU.AdditionalFiles["system/system_unused.h"] = guard(""); + TU.ExtraArgs = {"-isystem", testPath("system")}; + auto AST = TU.build(); + IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST, true); + EXPECT_THAT(Findings.UnusedIncludes, + ElementsAre(Pointee(writtenInclusion("<system_unused.h>")))); +} + TEST(IncludeCleaner, ComputeMissingHeaders) { Annotations MainFile(R"cpp( #include "a.h" diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 78b09d23d4427..3f1feea01fedb 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -81,6 +81,11 @@ Objective-C Miscellaneous ^^^^^^^^^^^^^ +- Added a boolean option `AnalyzeSystemHeaders` to `Includes` config section, + which allows to enable unused includes detection for all system headers. + At this moment umbrella headers are not supported, so enabling this option + may result in false-positives. + Improvements to clang-doc ------------------------- >From f66e5147bc3364c3ff2234013c02bb5497ae0b12 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin <vvd170...@gmail.com> Date: Sat, 11 May 2024 02:29:59 +0300 Subject: [PATCH 2/4] Move comment --- clang-tools-extra/clangd/Config.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index 9629854abff31..e6b461318365c 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -110,9 +110,9 @@ struct Config { IncludesPolicy UnusedIncludes = IncludesPolicy::Strict; IncludesPolicy MissingIncludes = IncludesPolicy::None; - /// IncludeCleaner will not diagnose usages of these headers matched by - /// these regexes. struct { + /// IncludeCleaner will not diagnose usages of these headers matched by + /// these regexes. std::vector<std::function<bool(llvm::StringRef)>> IgnoreHeader; bool AnalyzeSystemHeaders = false; } Includes; >From a0fb40666081af19302a41865726b21a4b20e410 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin <vvd170...@gmail.com> Date: Sat, 11 May 2024 02:37:13 +0300 Subject: [PATCH 3/4] Rename config field --- clang-tools-extra/clangd/Config.h | 2 +- clang-tools-extra/clangd/ConfigCompile.cpp | 14 +++++++------- clang-tools-extra/clangd/ConfigFragment.h | 2 +- clang-tools-extra/clangd/ConfigYAML.cpp | 6 +++--- clang-tools-extra/clangd/IncludeCleaner.cpp | 12 ++++++------ clang-tools-extra/clangd/IncludeCleaner.h | 2 +- clang-tools-extra/clangd/ParsedAST.cpp | 2 +- .../clangd/unittests/ConfigCompileTests.cpp | 6 +++--- .../clangd/unittests/ConfigYAMLTests.cpp | 6 +++--- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- 10 files changed, 28 insertions(+), 28 deletions(-) diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h index e6b461318365c..41143b9ebc8d2 100644 --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -114,7 +114,7 @@ struct Config { /// IncludeCleaner will not diagnose usages of these headers matched by /// these regexes. std::vector<std::function<bool(llvm::StringRef)>> IgnoreHeader; - bool AnalyzeSystemHeaders = false; + bool AnalyzeAngledIncludes = false; } Includes; } Diagnostics; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp index f74c870adfb73..1c6a11a6b935f 100644 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -591,13 +591,13 @@ struct FragmentCompiler { Filters->push_back(std::move(CompiledRegex)); } } - std::optional<bool> AnalyzeSystemHeaders; - if (F.AnalyzeSystemHeaders.has_value()) - AnalyzeSystemHeaders = **F.AnalyzeSystemHeaders; - if (!Filters && !AnalyzeSystemHeaders.has_value()) + std::optional<bool> AnalyzeAngledIncludes; + if (F.AnalyzeAngledIncludes.has_value()) + AnalyzeAngledIncludes = **F.AnalyzeAngledIncludes; + if (!Filters && !AnalyzeAngledIncludes.has_value()) return; Out.Apply.push_back([Filters = std::move(Filters), - AnalyzeSystemHeaders](const Params &, Config &C) { + AnalyzeAngledIncludes](const Params &, Config &C) { if (Filters) { auto Filter = [Filters](llvm::StringRef Path) { for (auto &Regex : *Filters) @@ -607,8 +607,8 @@ struct FragmentCompiler { }; C.Diagnostics.Includes.IgnoreHeader.emplace_back(std::move(Filter)); } - if (AnalyzeSystemHeaders.has_value()) - C.Diagnostics.Includes.AnalyzeSystemHeaders = *AnalyzeSystemHeaders; + if (AnalyzeAngledIncludes.has_value()) + C.Diagnostics.Includes.AnalyzeAngledIncludes = *AnalyzeAngledIncludes; }); } diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index ac8b88c245412..f3e51a9b6dbc4 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -257,7 +257,7 @@ struct Fragment { /// If false (default), unused system headers will be ignored. /// Standard library headers are analyzed regardless of this option. - std::optional<Located<bool>> AnalyzeSystemHeaders; + std::optional<Located<bool>> AnalyzeAngledIncludes; }; IncludesBlock Includes; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 7608af4200538..3e9b6a07d3b32 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -169,9 +169,9 @@ class Parser { if (auto Values = scalarValues(N)) F.IgnoreHeader = std::move(*Values); }); - Dict.handle("AnalyzeSystemHeaders", [&](Node &N) { - if (auto Value = boolValue(N, "AnalyzeSystemHeaders")) - F.AnalyzeSystemHeaders = *Value; + Dict.handle("AnalyzeAngledIncludes", [&](Node &N) { + if (auto Value = boolValue(N, "AnalyzeAngledIncludes")) + F.AnalyzeAngledIncludes = *Value; }); Dict.parse(N); } diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 0fdd0412d1e48..6afd389804f85 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -69,7 +69,7 @@ bool isIgnored(llvm::StringRef HeaderPath, HeaderFilter IgnoreHeaders) { bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, const include_cleaner::PragmaIncludes *PI, - bool AnalyzeSystemHeaders) { + bool AnalyzeAngledIncludes) { assert(Inc.HeaderID); auto HID = static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID); auto FE = AST.getSourceManager().getFileManager().getFileRef( @@ -90,7 +90,7 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, if (tooling::stdlib::Header::named(Inc.Written)) { return true; } - if (!AnalyzeSystemHeaders) { + if (!AnalyzeAngledIncludes) { return false; } } @@ -275,7 +275,7 @@ Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) { std::vector<const Inclusion *> getUnused(ParsedAST &AST, const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles, - bool AnalyzeSystemHeaders) { + bool AnalyzeAngledIncludes) { trace::Span Tracer("IncludeCleaner::getUnused"); std::vector<const Inclusion *> Unused; for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) { @@ -285,7 +285,7 @@ getUnused(ParsedAST &AST, if (ReferencedFiles.contains(IncludeID)) continue; if (!mayConsiderUnused(MFI, AST, &AST.getPragmaIncludes(), - AnalyzeSystemHeaders)) { + AnalyzeAngledIncludes)) { dlog("{0} was not used, but is not eligible to be diagnosed as unused", MFI.Written); continue; @@ -358,7 +358,7 @@ include_cleaner::Includes convertIncludes(const ParsedAST &AST) { } IncludeCleanerFindings -computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeSystemHeaders) { +computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeAngledIncludes) { // Interaction is only polished for C/CPP. if (AST.getLangOpts().ObjC) return {}; @@ -444,7 +444,7 @@ computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeSystemHeaders) { }); MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end()); std::vector<const Inclusion *> UnusedIncludes = - getUnused(AST, Used, AnalyzeSystemHeaders); + getUnused(AST, Used, AnalyzeAngledIncludes); return {std::move(UnusedIncludes), std::move(MissingIncludes)}; } diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index d03d74038b483..a01146d14e3c1 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -55,7 +55,7 @@ struct IncludeCleanerFindings { IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST, - bool AnalyzeSystemHeaders = false); + bool AnalyzeAngledIncludes = false); using HeaderFilter = llvm::ArrayRef<std::function<bool(llvm::StringRef)>>; std::vector<Diag> diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 8073b1ee76e64..2bd1fbcad2ada 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -374,7 +374,7 @@ std::vector<Diag> getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code, if (SuppressMissing && SuppressUnused) return {}; auto Findings = computeIncludeCleanerFindings( - AST, Cfg.Diagnostics.Includes.AnalyzeSystemHeaders); + AST, Cfg.Diagnostics.Includes.AnalyzeAngledIncludes); if (SuppressMissing) Findings.MissingIncludes.clear(); if (SuppressUnused) diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp index 20d873525fd16..4ecfdf0184ab4 100644 --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -279,10 +279,10 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) { EXPECT_FALSE(HeaderFilter("bar.h")); Frag = {}; - EXPECT_FALSE(Conf.Diagnostics.Includes.AnalyzeSystemHeaders); - Frag.Diagnostics.Includes.AnalyzeSystemHeaders = true; + EXPECT_FALSE(Conf.Diagnostics.Includes.AnalyzeAngledIncludes); + Frag.Diagnostics.Includes.AnalyzeAngledIncludes = true; EXPECT_TRUE(compileAndApply()); - EXPECT_TRUE(Conf.Diagnostics.Includes.AnalyzeSystemHeaders); + EXPECT_TRUE(Conf.Diagnostics.Includes.AnalyzeAngledIncludes); } TEST_F(ConfigCompileTests, DiagnosticSuppression) { diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp index 162db1fa34125..10d67dead342c 100644 --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -278,18 +278,18 @@ TEST(ParseYAML, IncludesIgnoreHeader) { ElementsAre(val("foo"), val("bar"))); } -TEST(ParseYAML, IncludesAnalyzeSystemHeaders) { +TEST(ParseYAML, IncludesAnalyzeAngledIncludes) { CapturedDiags Diags; Annotations YAML(R"yaml( Diagnostics: Includes: - AnalyzeSystemHeaders: true + AnalyzeAngledIncludes: true )yaml"); auto Results = Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); ASSERT_THAT(Diags.Diagnostics, IsEmpty()); ASSERT_EQ(Results.size(), 1u); - EXPECT_THAT(Results[0].Diagnostics.Includes.AnalyzeSystemHeaders, + EXPECT_THAT(Results[0].Diagnostics.Includes.AnalyzeAngledIncludes, llvm::ValueIs(val(true))); } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 3f1feea01fedb..da27d94dbd70a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -81,8 +81,8 @@ Objective-C Miscellaneous ^^^^^^^^^^^^^ -- Added a boolean option `AnalyzeSystemHeaders` to `Includes` config section, - which allows to enable unused includes detection for all system headers. +- Added a boolean option `AnalyzeAngledIncludes` to `Includes` config section, + which allows to enable unused includes detection for all angled ("system") headers. At this moment umbrella headers are not supported, so enabling this option may result in false-positives. >From 86562a1abfb0600f9903906b6ebee6dfb9017566 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin <vvd170...@gmail.com> Date: Sat, 11 May 2024 03:29:36 +0300 Subject: [PATCH 4/4] Update tests --- .../clangd/unittests/IncludeCleanerTests.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 104c42779383e..71c361e240334 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -108,6 +108,7 @@ TEST(IncludeCleaner, GetUnusedHeaders) { #include "unguarded.h" #include "unused.h" #include <system_header.h> + #include <non_system_angled_header.h> void foo() { a(); b(); @@ -122,6 +123,7 @@ TEST(IncludeCleaner, GetUnusedHeaders) { TU.AdditionalFiles["dir/c.h"] = guard("void c();"); TU.AdditionalFiles["unused.h"] = guard("void unused();"); TU.AdditionalFiles["dir/unused.h"] = guard("void dirUnused();"); + TU.AdditionalFiles["dir/non_system_angled_header.h"] = guard(""); TU.AdditionalFiles["system/system_header.h"] = guard(""); TU.AdditionalFiles["unguarded.h"] = ""; TU.ExtraArgs.push_back("-I" + testPath("dir")); @@ -135,19 +137,26 @@ TEST(IncludeCleaner, GetUnusedHeaders) { Pointee(writtenInclusion("\"dir/unused.h\"")))); } -TEST(IncludeCleaner, SystemUnusedHeaders) { +TEST(IncludeCleaner, UnusedAngledHeaders) { auto TU = TestTU::withCode(R"cpp( #include <system_header.h> #include <system_unused.h> + #include <non_system_angled_unused.h> SystemClass x; )cpp"); TU.AdditionalFiles["system/system_header.h"] = guard("class SystemClass {};"); TU.AdditionalFiles["system/system_unused.h"] = guard(""); - TU.ExtraArgs = {"-isystem", testPath("system")}; + TU.AdditionalFiles["dir/non_system_angled_unused.h"] = guard(""); + TU.ExtraArgs = { + "-isystem" + testPath("system"), + "-I" + testPath("dir"), + }; auto AST = TU.build(); IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST, true); EXPECT_THAT(Findings.UnusedIncludes, - ElementsAre(Pointee(writtenInclusion("<system_unused.h>")))); + UnorderedElementsAre( + Pointee(writtenInclusion("<system_unused.h>")), + Pointee(writtenInclusion("<non_system_angled_unused.h>")))); } TEST(IncludeCleaner, ComputeMissingHeaders) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits