[clang-tools-extra] bdf0b75 - [clangd] IncludeCleaner: Add filtering mechanism
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 19 05:56:44 PDT 2022
Author: Kirill Bobyrev
Date: 2022-04-19T14:56:27+02:00
New Revision: bdf0b757d5938a9f774d17e81be226da3229d3e5
URL: https://github.com/llvm/llvm-project/commit/bdf0b757d5938a9f774d17e81be226da3229d3e5
DIFF: https://github.com/llvm/llvm-project/commit/bdf0b757d5938a9f774d17e81be226da3229d3e5.diff
LOG: [clangd] IncludeCleaner: Add filtering mechanism
This introduces filtering out inclusions based on the resolved path. This
mechanism will be important for disabling warnings for headers that we can not
diagnose correctly yet.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D123488
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index c693aee56ce57..734dce43c5873 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -29,6 +29,8 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Regex.h"
+#include <functional>
#include <string>
#include <vector>
@@ -100,6 +102,12 @@ struct Config {
} ClangTidy;
UnusedIncludesPolicy UnusedIncludes = None;
+
+ /// IncludeCleaner will not diagnose usages of these headers matched by
+ /// these regexes.
+ struct {
+ std::vector<std::function<bool(llvm::StringRef)>> IgnoreHeader;
+ } Includes;
} Diagnostics;
/// Style of the codebase.
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 4438a29d56089..a4d7904781e4f 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -45,7 +45,9 @@
#include "llvm/Support/SMLoc.h"
#include "llvm/Support/SourceMgr.h"
#include <algorithm>
+#include <memory>
#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -432,6 +434,7 @@ struct FragmentCompiler {
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
+ compile(std::move(F.Includes));
compile(std::move(F.ClangTidy));
}
@@ -507,6 +510,41 @@ struct FragmentCompiler {
}
}
+ void compile(Fragment::DiagnosticsBlock::IncludesBlock &&F) {
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+ static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#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;
+ }
+ Filters->push_back(std::move(CompiledRegex));
+ }
+ if (Filters->empty())
+ 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);
+ });
+ }
+
void compile(Fragment::CompletionBlock &&F) {
if (F.AllScopes) {
Out.Apply.push_back(
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 3c574bace2221..34bff844cb26c 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -232,6 +232,15 @@ struct Fragment {
/// - None
llvm::Optional<Located<std::string>> UnusedIncludes;
+ /// Controls IncludeCleaner diagnostics.
+ struct IncludesBlock {
+ /// Regexes that will be used to avoid diagnosing certain includes as
+ /// unused or missing. These can match any suffix of the header file in
+ /// question.
+ std::vector<Located<std::string>> IgnoreHeader;
+ };
+ IncludesBlock Includes;
+
/// Controls how clang-tidy will run over the code base.
///
/// The settings are merged with any settings found in .clang-tidy
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 1e6e3cac8a484..e6c8d9e76ed67 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -23,10 +23,14 @@
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Syntax/Tokens.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include <functional>
namespace clang {
namespace clangd {
@@ -233,7 +237,8 @@ void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
}
}
-static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST) {
+static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
+ const Config &Cfg) {
if (Inc.BehindPragmaKeep)
return false;
@@ -258,6 +263,15 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST) {
FE->getName());
return false;
}
+ for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
+ // Convert the path to Unix slashes and try to match aginast the fiilter.
+ llvm::SmallString<64> Path(Inc.Resolved);
+ llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+ if (Filter(Inc.Resolved)) {
+ dlog("{0} header is filtered out by the configuration", FE->getName());
+ return false;
+ }
+ }
return true;
}
@@ -369,6 +383,7 @@ getUnused(ParsedAST &AST,
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
const llvm::StringSet<> &ReferencedPublicHeaders) {
trace::Span Tracer("IncludeCleaner::getUnused");
+ const Config &Cfg = Config::current();
std::vector<const Inclusion *> Unused;
for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
if (!MFI.HeaderID)
@@ -377,7 +392,7 @@ getUnused(ParsedAST &AST,
continue;
auto IncludeID = static_cast<IncludeStructure::HeaderID>(*MFI.HeaderID);
bool Used = ReferencedFiles.contains(IncludeID);
- if (!Used && !mayConsiderUnused(MFI, AST)) {
+ if (!Used && !mayConsiderUnused(MFI, AST, Cfg)) {
dlog("{0} was not used, but is not eligible to be diagnosed as unused",
MFI.Written);
continue;
diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 461218603d0ec..8761f371c1ad9 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -263,6 +263,24 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
Config::UnusedIncludesPolicy::Strict);
+
+ Frag = {};
+ EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
+ << Conf.Diagnostics.Includes.IgnoreHeader.size();
+ Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+ Located<std::string>("foo.h"));
+ Frag.Diagnostics.Includes.IgnoreHeader.push_back(
+ Located<std::string>(".*inc"));
+ EXPECT_TRUE(compileAndApply());
+ auto HeaderFilter = [this](llvm::StringRef Path) {
+ for (auto &Filter : Conf.Diagnostics.Includes.IgnoreHeader) {
+ if (Filter(Path))
+ return true;
+ }
+ return false;
+ };
+ EXPECT_TRUE(HeaderFilter("foo.h"));
+ EXPECT_FALSE(HeaderFilter("bar.h"));
}
TEST_F(ConfigCompileTests, DiagnosticSuppression) {
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 4aea1f06087c7..8e70759802fcd 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1738,6 +1738,8 @@ TEST(DiagnosticsTest, IncludeCleaner) {
]]
#include "used.h"
+ #include "ignore.h"
+
#include <system_header.h>
void foo() {
@@ -1754,12 +1756,19 @@ TEST(DiagnosticsTest, IncludeCleaner) {
#pragma once
void used() {}
)cpp";
+ TU.AdditionalFiles["ignore.h"] = R"cpp(
+ #pragma once
+ void ignore() {}
+ )cpp";
TU.AdditionalFiles["system/system_header.h"] = "";
TU.ExtraArgs = {"-isystem" + testPath("system")};
// Off by default.
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
Config Cfg;
Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+ // Set filtering.
+ Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back(
+ [](llvm::StringRef Header) { return Header.endswith("ignore.h"); });
WithContextValue WithCfg(Config::Key, std::move(Cfg));
EXPECT_THAT(
*TU.build().getDiagnostics(),
More information about the cfe-commits
mailing list