[clang-tools-extra] [include cleaner] Use tooling::HeaderIncludes for include insertions and deletions (PR #212852)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 3 07:06:33 PDT 2026
https://github.com/dmaclach updated https://github.com/llvm/llvm-project/pull/212852
>From 46e1e734e92c3b0174c7fe9372e604f7d37c2ab4 Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Wed, 29 Jul 2026 12:29:08 -0700
Subject: [PATCH 1/4] [include cleaner] Use tooling::HeaderIncludes for include
insertions and deletions
Instead of relying on magic UINT_MAX replacements for clang-format to resolve, this change uses tooling::HeaderIncludes to calculate precise offsets and replacement text for adding and removing headers.
---
.../include-cleaner/lib/Analysis.cpp | 33 +++++++++++++++----
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index e48a380211af0..a05c6145e6ca9 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -20,8 +20,10 @@
#include "clang/Lex/HeaderSearch.h"
#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/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
@@ -31,7 +33,6 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
#include <cassert>
-#include <climits>
#include <string>
namespace clang::include_cleaner {
@@ -168,12 +169,30 @@ std::string fixIncludes(const AnalysisResults &Results,
const format::FormatStyle &Style) {
assert(Style.isCpp() && "Only C++ style supports include insertions!");
tooling::Replacements R;
- // Encode insertions/deletions in the magic way clang-format understands.
- for (const Include *I : Results.Unused)
- cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 1, I->quote())));
- for (auto &[Spelled, _] : Results.Missing)
- cantFail(R.add(
- tooling::Replacement(FileName, UINT_MAX, 0, "#include " + Spelled)));
+ tooling::HeaderIncludes HeaderIncludes(FileName, Code, Style.IncludeStyle);
+
+ for (const Include *I : Results.Unused) {
+ auto Deletion = HeaderIncludes.remove(I->Spelled, I->Angled);
+ for (const auto &Del : Deletion) {
+ cantFail(R.add(Del));
+ }
+ }
+
+ llvm::DenseMap<unsigned, std::string> InsertionsByOffset;
+ for (auto &[Spelled, _] : Results.Missing) {
+ auto Insertion = HeaderIncludes.insert(StringRef{Spelled}.trim("\"<>"),
+ Spelled.starts_with('<'),
+ tooling::IncludeDirective::Include);
+ if (Insertion) {
+ InsertionsByOffset[Insertion->getOffset()] +=
+ Insertion->getReplacementText();
+ }
+ }
+
+ for (const auto &Entry : InsertionsByOffset) {
+ cantFail(
+ R.add(tooling::Replacement(FileName, Entry.first, 0, Entry.second)));
+ }
// "cleanup" actually turns the UINT_MAX replacements into concrete edits.
auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
return cantFail(tooling::applyAllReplacements(Code, Positioned));
>From 6f594801c695ce524d1bcc6207e24c1bbd6b4233 Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Wed, 29 Jul 2026 12:44:58 -0700
Subject: [PATCH 2/4] Removed comment that I missed.
---
clang-tools-extra/include-cleaner/lib/Analysis.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index a05c6145e6ca9..40d6ea6ac2f06 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -193,7 +193,6 @@ std::string fixIncludes(const AnalysisResults &Results,
cantFail(
R.add(tooling::Replacement(FileName, Entry.first, 0, Entry.second)));
}
- // "cleanup" actually turns the UINT_MAX replacements into concrete edits.
auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
return cantFail(tooling::applyAllReplacements(Code, Positioned));
}
>From 537b8fbc6945c8bc0641a7eb8cc13cba9f29b08a Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Wed, 29 Jul 2026 13:15:54 -0700
Subject: [PATCH 3/4] Replaced c++20 starts_with with a non c++20 replacement.
---
clang-tools-extra/include-cleaner/lib/Analysis.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 40d6ea6ac2f06..d6d4161678981 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -180,9 +180,9 @@ std::string fixIncludes(const AnalysisResults &Results,
llvm::DenseMap<unsigned, std::string> InsertionsByOffset;
for (auto &[Spelled, _] : Results.Missing) {
- auto Insertion = HeaderIncludes.insert(StringRef{Spelled}.trim("\"<>"),
- Spelled.starts_with('<'),
- tooling::IncludeDirective::Include);
+ auto Insertion = HeaderIncludes.insert(
+ StringRef{Spelled}.trim("\"<>"), !Spelled.empty() && Spelled[0] == '<',
+ tooling::IncludeDirective::Include);
if (Insertion) {
InsertionsByOffset[Insertion->getOffset()] +=
Insertion->getReplacementText();
>From d49a325eb14e3dcd5af290c3bfc4473399bff6cf Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Fri, 31 Jul 2026 13:53:40 -0700
Subject: [PATCH 4/4] Update with better policies for code with matching
offsets.
---
.../include-cleaner/lib/Analysis.cpp | 27 +++++++++++++++----
.../unittests/AnalysisTest.cpp | 13 +++++++++
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index d6d4161678981..60880252d1a41 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -164,6 +164,10 @@ analyze(llvm::ArrayRef<Decl *> ASTRoots,
return Results;
}
+bool isAngled(const std::string &String) {
+ return !String.empty() && String[0] == '<';
+}
+
std::string fixIncludes(const AnalysisResults &Results,
llvm::StringRef FileName, llvm::StringRef Code,
const format::FormatStyle &Style) {
@@ -178,20 +182,33 @@ std::string fixIncludes(const AnalysisResults &Results,
}
}
- llvm::DenseMap<unsigned, std::string> InsertionsByOffset;
+ struct InsertionInfo {
+ std::string Text;
+ unsigned Length = 0;
+ };
+ llvm::DenseMap<unsigned, InsertionInfo> InsertionsByOffset;
+
for (auto &[Spelled, _] : Results.Missing) {
auto Insertion = HeaderIncludes.insert(
- StringRef{Spelled}.trim("\"<>"), !Spelled.empty() && Spelled[0] == '<',
+ llvm::StringRef{Spelled}.trim("\"<>"), isAngled(Spelled),
tooling::IncludeDirective::Include);
if (Insertion) {
- InsertionsByOffset[Insertion->getOffset()] +=
- Insertion->getReplacementText();
+ auto &Info = InsertionsByOffset[Insertion->getOffset()];
+ Info.Text += Insertion->getReplacementText();
+ if (Insertion->getLength() > 0) {
+ // We can concatenate pure insertions (length 0), but at most one
+ // true replacement (length > 0) to avoid overwriting the length.
+ assert(Info.Length == 0 && "Multiple replacements at same offset?");
+ Info.Length = Insertion->getLength();
+ }
}
}
for (const auto &Entry : InsertionsByOffset) {
+ const auto &Info = Entry.second;
+ const unsigned Offset = Entry.first;
cantFail(
- R.add(tooling::Replacement(FileName, Entry.first, 0, Entry.second)));
+ R.add(tooling::Replacement(FileName, Offset, Info.Length, Info.Text)));
}
auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
return cantFail(tooling::applyAllReplacements(Code, Positioned));
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index ba5a3fbbcaeb2..95d700b8764b7 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -489,6 +489,19 @@ R"cpp(#include "d.h"
#include "a.h")cpp");
}
+TEST(FixIncludes, MultipleInsertionsSameOffset) {
+ AnalysisResults Results;
+ Results.Missing.emplace_back("\"a.h\"", Header(""));
+ Results.Missing.emplace_back("\"b.h\"", Header(""));
+
+ // Empty code guarantees HeaderIncludes chooses offset 0 for both.
+ llvm::StringRef Code = "";
+
+ // Should concatenate them without conflict errors in Replacements::add
+ EXPECT_EQ(fixIncludes(Results, "d.cc", Code, format::getLLVMStyle()),
+ "#include \"a.h\"\n#include \"b.h\"\n");
+}
+
MATCHER_P3(expandedAt, FileID, Offset, SM, "") {
auto [ExpanedFileID, ExpandedOffset] = SM->getDecomposedExpansionLoc(arg);
return ExpanedFileID == FileID && ExpandedOffset == Offset;
More information about the cfe-commits
mailing list