[clang-tools-extra] [clang-tidy][IncludeCleaner] Fix analysis supression in presence of verbatim spellings (PR #68185)
kadir çetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 4 01:56:57 PDT 2023
https://github.com/kadircet updated https://github.com/llvm/llvm-project/pull/68185
>From 91ce80c89689518c20f18efd172628a51aebb769 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Wed, 4 Oct 2023 09:38:28 +0200
Subject: [PATCH 1/2] [clang-tidy][IncludeCleaner] Fix analysis supression in
presence of verbatim spellings
---
.../clang-tidy/misc/IncludeCleanerCheck.cpp | 20 ++++++++++++-------
.../clang-tidy/IncludeCleanerTest.cpp | 16 ++++++++++++---
2 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index fed19bdcc291436..a95a1feb9852b99 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -30,6 +30,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Inclusions/HeaderIncludes.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
@@ -97,9 +98,12 @@ bool IncludeCleanerCheck::shouldIgnore(const include_cleaner::Header &H) {
return llvm::any_of(IgnoreHeadersRegex, [&H](const llvm::Regex &R) {
switch (H.kind()) {
case include_cleaner::Header::Standard:
+ // We don't trim braces around standard library headers deliberately, so
+ // that they are only matched as <vector>, otherwise having just
+ // `.*/vector` might yield false positives.
return R.match(H.standard().name());
case include_cleaner::Header::Verbatim:
- return R.match(H.verbatim());
+ return R.match(H.verbatim().trim("<>\""));
case include_cleaner::Header::Physical:
return R.match(H.physical().getFileEntry().tryGetRealPathName());
}
@@ -179,12 +183,14 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
if (getCurrentMainFile().endswith(PHeader))
continue;
}
-
- if (llvm::none_of(
- IgnoreHeadersRegex,
- [Resolved = (*I.Resolved).getFileEntry().tryGetRealPathName()](
- const llvm::Regex &R) { return R.match(Resolved); }))
- Unused.push_back(&I);
+ auto StdHeader = tooling::stdlib::Header::named(
+ I.quote(), PP->getLangOpts().CPlusPlus ? tooling::stdlib::Lang::CXX
+ : tooling::stdlib::Lang::C);
+ if (StdHeader && shouldIgnore(*StdHeader))
+ continue;
+ if (shouldIgnore(*I.Resolved))
+ continue;
+ Unused.push_back(&I);
}
llvm::StringRef Code = SM->getBufferData(SM->getMainFileID());
diff --git a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
index 0c29b469b617b7c..8da1051a860a8c7 100644
--- a/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp
@@ -59,18 +59,20 @@ TEST(IncludeCleanerCheckTest, SuppressUnusedIncludes) {
#include "foo/qux.h"
#include "baz/qux/qux.h"
#include <vector>
+#include <list>
)";
const char *PostCode = R"(
#include "bar.h"
#include "foo/qux.h"
#include <vector>
+#include <list>
)";
std::vector<ClangTidyError> Errors;
ClangTidyOptions Opts;
Opts.CheckOptions["IgnoreHeaders"] = llvm::StringRef{llvm::formatv(
- "bar.h;{0};{1};vector",
+ "bar.h;{0};{1};vector;<list>;",
llvm::Regex::escape(appendPathFileSystemIndependent({"foo", "qux.h"})),
llvm::Regex::escape(appendPathFileSystemIndependent({"baz", "qux"})))};
EXPECT_EQ(
@@ -79,6 +81,7 @@ TEST(IncludeCleanerCheckTest, SuppressUnusedIncludes) {
PreCode, &Errors, "file.cpp", std::nullopt, Opts,
{{"bar.h", "#pragma once"},
{"vector", "#pragma once"},
+ {"list", "#pragma once"},
{appendPathFileSystemIndependent({"foo", "qux.h"}), "#pragma once"},
{appendPathFileSystemIndependent({"baz", "qux", "qux.h"}),
"#pragma once"}}));
@@ -163,11 +166,13 @@ TEST(IncludeCleanerCheckTest, SuppressMissingIncludes) {
int BarResult = bar();
int BazResult = baz();
int QuxResult = qux();
+int PrivResult = test();
+std::vector x;
)";
ClangTidyOptions Opts;
Opts.CheckOptions["IgnoreHeaders"] = llvm::StringRef{
- "baz.h;" +
+ "public.h;<vector>;baz.h;" +
llvm::Regex::escape(appendPathFileSystemIndependent({"foo", "qux.h"}))};
std::vector<ClangTidyError> Errors;
EXPECT_EQ(PreCode, runCheckOnCode<IncludeCleanerCheck>(
@@ -175,18 +180,23 @@ int QuxResult = qux();
{{"bar.h", R"(#pragma once
#include "baz.h"
#include "foo/qux.h"
+ #include "private.h"
int bar();
+ namespace std { struct vector {}; }
)"},
{"baz.h", R"(#pragma once
int baz();
)"},
+ {"private.h", R"(#pragma once
+ // IWYU pragma: private, include "public.h"
+ int test();
+ )"},
{appendPathFileSystemIndependent({"foo", "qux.h"}),
R"(#pragma once
int qux();
)"}}));
}
-
TEST(IncludeCleanerCheckTest, MultipleTimeMissingInclude) {
const char *PreCode = R"(
#include "bar.h"
>From 22f7e180bd22c6f33a83399c7ca28c1fd3a2084e Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Wed, 4 Oct 2023 10:56:36 +0200
Subject: [PATCH 2/2] Update comment
---
clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
index a95a1feb9852b99..e336ba1ee1fa729 100644
--- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
@@ -98,9 +98,9 @@ bool IncludeCleanerCheck::shouldIgnore(const include_cleaner::Header &H) {
return llvm::any_of(IgnoreHeadersRegex, [&H](const llvm::Regex &R) {
switch (H.kind()) {
case include_cleaner::Header::Standard:
- // We don't trim braces around standard library headers deliberately, so
- // that they are only matched as <vector>, otherwise having just
- // `.*/vector` might yield false positives.
+ // We don't trim angle brackets around standard library headers
+ // deliberately, so that they are only matched as <vector>, otherwise
+ // having just `.*/vector` might yield false positives.
return R.match(H.standard().name());
case include_cleaner::Header::Verbatim:
return R.match(H.verbatim().trim("<>\""));
More information about the cfe-commits
mailing list