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/6] 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/6] 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/6] 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/6] 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) {
>From a29e25897ab8b7815bf12b283ce391a64f2db6e9 Mon Sep 17 00:00:00 2001
From: Vadim Dudkin <vvd170...@gmail.com>
Date: Mon, 13 May 2024 13:09:54 +0300
Subject: [PATCH 5/6] Add comment, fix style
---
clang-tools-extra/clangd/ConfigCompile.cpp | 3 +++
clang-tools-extra/clangd/IncludeCleaner.cpp | 6 ++----
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 1c6a11a6b935f..f32f674443ffe 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -591,6 +591,9 @@ struct FragmentCompiler {
Filters->push_back(std::move(CompiledRegex));
}
}
+ // Optional to override the resulting AnalyzeAngledIncludes
+ // only if it's explicitly set in the current fragment.
+ // Otherwise it's inherited from parent fragment.
std::optional<bool> AnalyzeAngledIncludes;
if (F.AnalyzeAngledIncludes.has_value())
AnalyzeAngledIncludes = **F.AnalyzeAngledIncludes;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 6afd389804f85..01b47679790f1 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -87,12 +87,10 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
// 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)) {
+ if (tooling::stdlib::Header::named(Inc.Written))
return true;
- }
- if (!AnalyzeAngledIncludes) {
+ if (!AnalyzeAngledIncludes)
return false;
- }
}
if (PI) {
// Check if main file is the public interface for a private header. If so
we
>From bd48f0aadc95dfd6e6d20847d894b2d39f8f2cad Mon Sep 17 00:00:00 2001
From: Vadim Dudkin <vvd170...@gmail.com>
Date: Mon, 13 May 2024 13:12:11 +0300
Subject: [PATCH 6/6] Add test for ignored angled includes
---
.../clangd/unittests/IncludeCleanerTests.cpp | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index 71c361e240334..7027232460354 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -137,6 +137,26 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
Pointee(writtenInclusion("\"dir/unused.h\""))));
}
+TEST(IncludeCleaner, IgnoredAngledHeaders) {
+ // Currently the default behavior is to ignore unused angled includes
+ 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.AdditionalFiles["dir/non_system_angled_unused.h"] = guard("");
+ TU.ExtraArgs = {
+ "-isystem" + testPath("system"),
+ "-I" + testPath("dir"),
+ };
+ auto AST = TU.build();
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+}
+
TEST(IncludeCleaner, UnusedAngledHeaders) {
auto TU = TestTU::withCode(R"cpp(
#include <system_header.h>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
