[clang] [clang-tools-extra] [include cleaner] Use tooling::HeaderIncludes for include insertions and deletions (PR #212852)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 25 14:53:10 PDT 2026
https://github.com/dmaclach updated https://github.com/llvm/llvm-project/pull/212852
>From 4c545d19338d660eb6101207ee701dac2440afab 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/6] [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 922a690dfc47c..2cbde460b4ba5 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>
#include <utility>
@@ -167,12 +168,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 ca539d937294de60b727ac57dddf217f7191668f 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/6] 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 2cbde460b4ba5..de194fbb3366a 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -192,7 +192,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 f176cfb128843a51397ed20ad24841f61ea090c3 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/6] 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 de194fbb3366a..609105566ce22 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -179,9 +179,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 ec61f1dc4e559867801947cdf2e7808821b42da7 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/6] 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 609105566ce22..99e7375553e2f 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -163,6 +163,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) {
@@ -177,20 +181,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;
>From 290cb6ad54c2e1cef8d3a007416350052010d3d5 Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Mon, 17 Aug 2026 12:58:17 -0700
Subject: [PATCH 5/6] Move batch insertion logic into HeaderIncludes for
potential reuse and locality. Add more tests as requested.
---
.../include-cleaner/lib/Analysis.cpp | 34 ++------
.../unittests/AnalysisTest.cpp | 68 ++++++++++++++-
.../clang/Tooling/Inclusions/HeaderIncludes.h | 46 ++++++++--
.../lib/Tooling/Inclusions/HeaderIncludes.cpp | 85 ++++++++++++++++++-
4 files changed, 190 insertions(+), 43 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 99e7375553e2f..b1b055ee4728c 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -163,10 +163,6 @@ 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) {
@@ -181,33 +177,13 @@ std::string fixIncludes(const AnalysisResults &Results,
}
}
- struct InsertionInfo {
- std::string Text;
- unsigned Length = 0;
- };
- llvm::DenseMap<unsigned, InsertionInfo> InsertionsByOffset;
-
- for (auto &[Spelled, _] : Results.Missing) {
- auto Insertion = HeaderIncludes.insert(
- llvm::StringRef{Spelled}.trim("\"<>"), isAngled(Spelled),
- tooling::IncludeDirective::Include);
- if (Insertion) {
- 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();
- }
- }
+ llvm::SmallVector<tooling::HeaderIncludes::HeaderToInsert> HeadersToInsert;
+ for (const auto &[Spelled, _] : Results.Missing) {
+ HeadersToInsert.emplace_back(Spelled, tooling::IncludeDirective::Include);
}
- for (const auto &Entry : InsertionsByOffset) {
- const auto &Info = Entry.second;
- const unsigned Offset = Entry.first;
- cantFail(
- R.add(tooling::Replacement(FileName, Offset, Info.Length, Info.Text)));
+ for (const auto &Repl : HeaderIncludes.insert(HeadersToInsert)) {
+ cantFail(R.add(Repl));
}
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 95d700b8764b7..87ffd7e6cf3c3 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -26,7 +26,6 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Testing/Annotations/Annotations.h"
@@ -398,7 +397,7 @@ TEST_F(AnalyzeTest, SpellingIncludesWithSymlinks) {
}
// Make sure that the references to implicit operator new/delete are reported as
-// ambigious.
+// ambiguous.
TEST_F(AnalyzeTest, ImplicitOperatorNewDeleteNotMissing) {
ExtraFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
ExtraFS->addFile("header.h",
@@ -719,5 +718,70 @@ TEST_F(WalkUsedTest, MacroConcat) {
AllOf(Contains(Pair(Code.point("bar"), UnorderedElementsAre(Header))),
Contains(Pair(Code.point("xyz"), UnorderedElementsAre(Header)))));
}
+
+TEST(FixIncludes, MissingIncludesSortingAndGrouping) {
+ AnalysisResults Results;
+ Results.Missing.push_back({"\"b.h\"", Header("\"b.h\"")});
+ Results.Missing.push_back({"\"a.h\"", Header("\"a.h\"")});
+ Results.Missing.push_back({"<foo>", Header("<foo>")});
+
+ format::FormatStyle Style = format::getLLVMStyle();
+ Style.Language = format::FormatStyle::LK_Cpp;
+
+ std::string Code = R"cpp(
+void bar();
+)cpp";
+
+ std::string Fixed = fixIncludes(Results, "test.cc", Code, Style);
+ EXPECT_EQ(
+ Fixed,
+ "\n#include \"a.h\"\n#include \"b.h\"\n#include <foo>\nvoid bar();\n");
+}
+
+TEST(FixIncludes, MainHeaderGrouping) {
+ AnalysisResults Results;
+ Results.Missing.push_back({"\"b.h\"", Header("\"b.h\"")});
+ Results.Missing.push_back({"\"foo.h\"", Header("\"foo.h\"")});
+ Results.Missing.push_back({"\"a.h\"", Header("\"a.h\"")});
+ Results.Missing.push_back({"<vector>", Header("<vector>")});
+
+ format::FormatStyle Style = format::getLLVMStyle();
+ Style.Language = format::FormatStyle::LK_Cpp;
+
+ std::string Code = R"cpp(
+void test();
+)cpp";
+
+ std::string Fixed = fixIncludes(Results, "foo.cc", Code, Style);
+ EXPECT_EQ(Fixed, "\n#include \"foo.h\"\n#include \"a.h\"\n#include "
+ "\"b.h\"\n#include <vector>\nvoid test();\n");
+}
+
+TEST(FixIncludes, MultipleInsertionsAndDeletions) {
+ AnalysisResults Results;
+ Include UnusedInc;
+ UnusedInc.Spelled = "unused.h";
+ UnusedInc.Line = 1;
+ Results.Unused.push_back(&UnusedInc);
+
+ Results.Missing.push_back({"\"a.h\"", Header("\"a.h\"")});
+ Results.Missing.push_back({"<foo>", Header("<foo>")});
+
+ format::FormatStyle Style = format::getLLVMStyle();
+ Style.Language = format::FormatStyle::LK_Cpp;
+
+ std::string Code = R"cpp(#include "unused.h"
+
+void test();
+)cpp";
+
+ std::string Fixed = fixIncludes(Results, "test.cc", Code, Style);
+ EXPECT_EQ(Fixed, R"cpp(#include "a.h"
+#include <foo>
+
+void test();
+)cpp");
+}
+
} // namespace
} // namespace clang::include_cleaner
diff --git a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
index 72407e2b12062..21ef0a171fea8 100644
--- a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -9,13 +9,15 @@
#ifndef LLVM_CLANG_TOOLING_INCLUSIONS_HEADERINCLUDES_H
#define LLVM_CLANG_TOOLING_INCLUSIONS_HEADERINCLUDES_H
-#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Inclusions/IncludeStyle.h"
-#include "llvm/Support/Path.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Regex.h"
#include <list>
#include <optional>
+#include <set>
+#include <string>
#include <unordered_map>
namespace clang {
@@ -51,8 +53,7 @@ enum class IncludeDirective { Include, Import };
/// file.
class HeaderIncludes {
public:
- HeaderIncludes(llvm::StringRef FileName, llvm::StringRef Code,
- const IncludeStyle &Style);
+ HeaderIncludes(StringRef FileName, StringRef Code, const IncludeStyle &Style);
/// Inserts an #include or #import directive of \p Header into the code.
/// If \p IsAngled is true, \p Header will be quoted with <> in the directive;
@@ -73,15 +74,43 @@ class HeaderIncludes {
/// same category in the code that should be sorted after \p IncludeName. If
/// \p IncludeName already exists (with exactly the same spelling), this
/// returns std::nullopt.
- std::optional<tooling::Replacement> insert(llvm::StringRef Header,
- bool IsAngled,
+ std::optional<tooling::Replacement> insert(StringRef Header, bool IsAngled,
IncludeDirective Directive) const;
+ /// Represents a single header directive to be inserted in a batch operation.
+ ///
+ /// Usage:
+ /// - HeaderToInsert("<vector>") -> inserts #include <vector>
+ /// (auto-detects angled)
+ /// - HeaderToInsert("\"foo.h\"") -> inserts #include "foo.h"
+ /// (auto-detects quoted)
+ /// - HeaderToInsert("<foo>", IncludeDirective::Import) -> inserts #import
+ /// <foo>
+ /// - HeaderToInsert("foo.h", IncludeDirective::Include, /*IsAngled=*/false)
+ /// -> explicit IsAngled
+ struct HeaderToInsert {
+ // The header name, with any surrounding quotes or brackets removed.
+ std::string Header;
+ // Whether to insert #include or #import.
+ IncludeDirective Directive;
+ // Whether to use <> or "" for the header. If not set, the default is
+ // determined by the header name.
+ bool IsAngled;
+
+ HeaderToInsert(StringRef RawOrSpelledHeader,
+ IncludeDirective Directive = IncludeDirective::Include,
+ std::optional<bool> IsAngled = std::nullopt);
+ };
+
+ /// Inserts a batch of headers into the code, sorting and grouping them
+ /// according to IncludeStyle and returning the replacements.
+ tooling::Replacements insert(ArrayRef<HeaderToInsert> Headers) const;
+
/// Removes all existing #includes and #imports of \p Header quoted with <> if
/// \p IsAngled is true or "" if \p IsAngled is false.
/// This doesn't resolve the header file path; it only deletes #includes and
/// #imports with exactly the same spelling.
- tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
+ tooling::Replacements remove(StringRef Header, bool IsAngled) const;
// Matches a whole #include directive.
static const llvm::Regex IncludeRegex;
@@ -117,8 +146,7 @@ class HeaderIncludes {
/// in the order they appear in the source file.
/// See comment for "FormatStyle::IncludeCategories" for details about include
/// priorities.
- std::unordered_map<int, llvm::SmallVector<const Include *, 8>>
- IncludesByPriority;
+ std::unordered_map<int, SmallVector<const Include *, 8>> IncludesByPriority;
int FirstIncludeOffset;
// All new headers should be inserted after this offset (e.g. after header
diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index c3bbf6b5f2e73..09e2a64576a67 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -14,6 +14,8 @@
#include "clang/Lex/Token.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Inclusions/IncludeStyle.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -25,10 +27,12 @@
#include <cassert>
#include <climits>
#include <functional>
+#include <iterator>
#include <optional>
#include <string>
#include <type_traits>
#include <utility>
+#include <vector>
namespace clang {
namespace tooling {
@@ -512,11 +516,86 @@ HeaderIncludes::insert(llvm::StringRef Header, bool IsAngled,
return tooling::Replacement(FileName, InsertOffset, 0, NewInclude);
}
-tooling::Replacements HeaderIncludes::remove(llvm::StringRef IncludeName,
+HeaderIncludes::HeaderToInsert::HeaderToInsert(StringRef RawOrSpelledHeader,
+ IncludeDirective Directive,
+ std::optional<bool> IsAngled)
+ : Directive(Directive) {
+ if (RawOrSpelledHeader.starts_with("<")) {
+ Header = RawOrSpelledHeader.trim("<>").str();
+ this->IsAngled = IsAngled.value_or(true);
+ } else if (RawOrSpelledHeader.starts_with("\"")) {
+ Header = RawOrSpelledHeader.trim("\"").str();
+ this->IsAngled = IsAngled.value_or(false);
+ } else {
+ Header = RawOrSpelledHeader.str();
+ this->IsAngled = IsAngled.value_or(false);
+ }
+}
+
+tooling::Replacements
+HeaderIncludes::insert(llvm::ArrayRef<HeaderToInsert> Headers) const {
+ tooling::Replacements Result;
+ if (Headers.empty())
+ return Result;
+
+ std::vector<HeaderToInsert> SortedHeaders = Headers.vec();
+ llvm::stable_sort(SortedHeaders, [&](const HeaderToInsert &L,
+ const HeaderToInsert &R) {
+ std::string QuotedL =
+ std::string(llvm::formatv(L.IsAngled ? "<{0}>" : "\"{0}\"", L.Header));
+ std::string QuotedR =
+ std::string(llvm::formatv(R.IsAngled ? "<{0}>" : "\"{0}\"", R.Header));
+ int PriorityL = Categories.getIncludePriority(
+ QuotedL, /*CheckMainHeader=*/!MainIncludeFound);
+ int PriorityR = Categories.getIncludePriority(
+ QuotedR, /*CheckMainHeader=*/!MainIncludeFound);
+ if (PriorityL != PriorityR)
+ return PriorityL < PriorityR;
+ if (L.Header != R.Header)
+ return L.Header < R.Header;
+ if (L.IsAngled != R.IsAngled)
+ return L.IsAngled < R.IsAngled;
+ return L.Directive > R.Directive;
+ });
+ SortedHeaders.erase(
+ std::unique(SortedHeaders.begin(), SortedHeaders.end(),
+ [](const HeaderToInsert &L, const HeaderToInsert &R) {
+ return L.Header == R.Header && L.IsAngled == R.IsAngled;
+ }),
+ SortedHeaders.end());
+
+ struct InsertionInfo {
+ std::string Text;
+ unsigned Length = 0;
+ };
+ llvm::DenseMap<unsigned, InsertionInfo> InsertionsByOffset;
+
+ for (const auto &H : SortedHeaders) {
+ if (auto Insertion = insert(H.Header, H.IsAngled, H.Directive)) {
+ auto &Info = InsertionsByOffset[Insertion->getOffset()];
+ Info.Text += Insertion->getReplacementText();
+ if (Insertion->getLength() > 0) {
+ 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(Result.add(
+ tooling::Replacement(FileName, Offset, Info.Length, Info.Text)));
+ }
+
+ return Result;
+}
+
+tooling::Replacements HeaderIncludes::remove(llvm::StringRef Header,
bool IsAngled) const {
- assert(IncludeName == trimInclude(IncludeName));
+ assert(Header == trimInclude(Header));
tooling::Replacements Result;
- auto Iter = ExistingIncludes.find(IncludeName);
+ auto Iter = ExistingIncludes.find(Header);
if (Iter == ExistingIncludes.end())
return Result;
for (const auto &Inc : Iter->second) {
>From 7375bd5d98ebe677e4fb50b4d39b4c768676ae2f Mon Sep 17 00:00:00 2001
From: Dave MacLachlan <dmaclach at gmail.com>
Date: Tue, 25 Aug 2026 14:51:15 -0700
Subject: [PATCH 6/6] Responding to comments: - Restored `llvm::` prefixes
- Added enum - Updated documentation - Made Format.cpp use the new bulk
insertion function from HeaderIncludes. - Restored Analyze.cpp back to
original calling through Format.cpp
---
.../unittests/AnalysisTest.cpp | 28 ++++++++---------
.../clang/Tooling/Inclusions/HeaderIncludes.h | 31 ++++++++++++-------
clang/lib/Format/Format.cpp | 25 +++++++--------
.../lib/Tooling/Inclusions/HeaderIncludes.cpp | 13 +++-----
4 files changed, 50 insertions(+), 47 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 87ffd7e6cf3c3..5d2643367c5bb 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -488,19 +488,6 @@ 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;
@@ -742,8 +729,8 @@ TEST(FixIncludes, MainHeaderGrouping) {
AnalysisResults Results;
Results.Missing.push_back({"\"b.h\"", Header("\"b.h\"")});
Results.Missing.push_back({"\"foo.h\"", Header("\"foo.h\"")});
- Results.Missing.push_back({"\"a.h\"", Header("\"a.h\"")});
Results.Missing.push_back({"<vector>", Header("<vector>")});
+ Results.Missing.push_back({"\"a.h\"", Header("\"a.h\"")});
format::FormatStyle Style = format::getLLVMStyle();
Style.Language = format::FormatStyle::LK_Cpp;
@@ -783,5 +770,18 @@ void test();
)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");
+}
+
} // namespace
} // namespace clang::include_cleaner
diff --git a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
index 21ef0a171fea8..84b1d19e52c8a 100644
--- a/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ b/clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -9,9 +9,10 @@
#ifndef LLVM_CLANG_TOOLING_INCLUSIONS_HEADERINCLUDES_H
#define LLVM_CLANG_TOOLING_INCLUSIONS_HEADERINCLUDES_H
-#include "clang/Basic/LLVM.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Inclusions/IncludeStyle.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Regex.h"
#include <list>
@@ -53,7 +54,8 @@ enum class IncludeDirective { Include, Import };
/// file.
class HeaderIncludes {
public:
- HeaderIncludes(StringRef FileName, StringRef Code, const IncludeStyle &Style);
+ HeaderIncludes(llvm::StringRef FileName, llvm::StringRef Code,
+ const IncludeStyle &Style);
/// Inserts an #include or #import directive of \p Header into the code.
/// If \p IsAngled is true, \p Header will be quoted with <> in the directive;
@@ -65,7 +67,7 @@ class HeaderIncludes {
/// default. These code sections include:
/// - raw string literals (containing #include).
/// - #if blocks.
- /// - Special #include's among declarations (e.g. functions).
+ /// - Special #includes among declarations (e.g. functions).
///
/// Returns a replacement that inserts the new header into a suitable #include
/// block of the same category. This respects the order of the existing
@@ -74,7 +76,8 @@ class HeaderIncludes {
/// same category in the code that should be sorted after \p IncludeName. If
/// \p IncludeName already exists (with exactly the same spelling), this
/// returns std::nullopt.
- std::optional<tooling::Replacement> insert(StringRef Header, bool IsAngled,
+ std::optional<tooling::Replacement> insert(llvm::StringRef Header,
+ bool IsAngled,
IncludeDirective Directive) const;
/// Represents a single header directive to be inserted in a batch operation.
@@ -89,35 +92,38 @@ class HeaderIncludes {
/// - HeaderToInsert("foo.h", IncludeDirective::Include, /*IsAngled=*/false)
/// -> explicit IsAngled
struct HeaderToInsert {
+ enum class QuoteStyle { AUTO, ANGLED, QUOTED };
+
// The header name, with any surrounding quotes or brackets removed.
std::string Header;
// Whether to insert #include or #import.
IncludeDirective Directive;
- // Whether to use <> or "" for the header. If not set, the default is
- // determined by the header name.
+ // Whether to use <> or "" for the header. This can be set explicitly with
+ // QuoteStyle::ANGLED or QuoteStyle::QUOTED, or auto-detected based on
+ // `RawOrSpelledHeader` with QuoteStyle::AUTO.
bool IsAngled;
- HeaderToInsert(StringRef RawOrSpelledHeader,
+ HeaderToInsert(llvm::StringRef RawOrSpelledHeader,
IncludeDirective Directive = IncludeDirective::Include,
- std::optional<bool> IsAngled = std::nullopt);
+ QuoteStyle QuoteStyle = QuoteStyle::AUTO);
};
/// Inserts a batch of headers into the code, sorting and grouping them
/// according to IncludeStyle and returning the replacements.
- tooling::Replacements insert(ArrayRef<HeaderToInsert> Headers) const;
+ tooling::Replacements insert(llvm::ArrayRef<HeaderToInsert> Headers) const;
/// Removes all existing #includes and #imports of \p Header quoted with <> if
/// \p IsAngled is true or "" if \p IsAngled is false.
/// This doesn't resolve the header file path; it only deletes #includes and
/// #imports with exactly the same spelling.
- tooling::Replacements remove(StringRef Header, bool IsAngled) const;
+ tooling::Replacements remove(llvm::StringRef Header, bool IsAngled) const;
// Matches a whole #include directive.
static const llvm::Regex IncludeRegex;
private:
struct Include {
- Include(StringRef Name, tooling::Range R, IncludeDirective D)
+ Include(llvm::StringRef Name, tooling::Range R, IncludeDirective D)
: Name(Name), R(R), Directive(D) {}
// An include header quoted with either <> or "".
@@ -146,7 +152,8 @@ class HeaderIncludes {
/// in the order they appear in the source file.
/// See comment for "FormatStyle::IncludeCategories" for details about include
/// priorities.
- std::unordered_map<int, SmallVector<const Include *, 8>> IncludesByPriority;
+ std::unordered_map<int, llvm::SmallVector<const Include *, 8>>
+ IncludesByPriority;
int FirstIncludeOffset;
// All new headers should be inserted after this offset (e.g. after header
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index e5533c32899a3..b39d64664d671 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -4211,6 +4211,7 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
}
SmallVector<StringRef, 4> Matches;
+ SmallVector<tooling::HeaderIncludes::HeaderToInsert, 4> HeadersToInsert;
for (const auto &R : HeaderInsertions) {
auto IncludeDirective = R.getReplacementText();
bool Matched =
@@ -4219,19 +4220,17 @@ fixCppIncludeInsertions(StringRef Code, const tooling::Replacements &Replaces,
"'#include ...'");
(void)Matched;
auto IncludeName = Matches[2];
- auto Replace =
- Includes.insert(IncludeName.trim("\"<>"), IncludeName.starts_with("<"),
- tooling::IncludeDirective::Include);
- if (Replace) {
- auto Err = Result.add(*Replace);
- if (Err) {
- consumeError(std::move(Err));
- unsigned NewOffset =
- Result.getShiftedCodePosition(Replace->getOffset());
- auto Shifted = tooling::Replacement(FileName, NewOffset, 0,
- Replace->getReplacementText());
- Result = Result.merge(tooling::Replacements(Shifted));
- }
+ HeadersToInsert.emplace_back(IncludeName,
+ tooling::IncludeDirective::Include);
+ }
+ for (const auto &Replace : Includes.insert(HeadersToInsert)) {
+ auto Err = Result.add(Replace);
+ if (Err) {
+ consumeError(std::move(Err));
+ unsigned NewOffset = Result.getShiftedCodePosition(Replace.getOffset());
+ auto Shifted = tooling::Replacement(FileName, NewOffset, 0,
+ Replace.getReplacementText());
+ Result = Result.merge(tooling::Replacements(Shifted));
}
}
return Result;
diff --git a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
index 09e2a64576a67..285d4a40157db 100644
--- a/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ b/clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -516,19 +516,16 @@ HeaderIncludes::insert(llvm::StringRef Header, bool IsAngled,
return tooling::Replacement(FileName, InsertOffset, 0, NewInclude);
}
-HeaderIncludes::HeaderToInsert::HeaderToInsert(StringRef RawOrSpelledHeader,
- IncludeDirective Directive,
- std::optional<bool> IsAngled)
+HeaderIncludes::HeaderToInsert::HeaderToInsert(
+ llvm::StringRef RawOrSpelledHeader, IncludeDirective Directive,
+ QuoteStyle QuoteStyle)
: Directive(Directive) {
if (RawOrSpelledHeader.starts_with("<")) {
Header = RawOrSpelledHeader.trim("<>").str();
- this->IsAngled = IsAngled.value_or(true);
+ this->IsAngled = QuoteStyle != QuoteStyle::QUOTED;
} else if (RawOrSpelledHeader.starts_with("\"")) {
Header = RawOrSpelledHeader.trim("\"").str();
- this->IsAngled = IsAngled.value_or(false);
- } else {
- Header = RawOrSpelledHeader.str();
- this->IsAngled = IsAngled.value_or(false);
+ this->IsAngled = QuoteStyle == QuoteStyle::ANGLED;
}
}
More information about the cfe-commits
mailing list