From 4c545d19338d660eb6101207ee701dac2440afab Mon Sep 17 00:00:00 2001 From: Dave MacLachlan Date: Wed, 29 Jul 2026 12:29:08 -0700 Subject: [PATCH 1/7] [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 -#include #include #include @@ -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 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 Date: Wed, 29 Jul 2026 12:44:58 -0700 Subject: [PATCH 2/7] 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 Date: Wed, 29 Jul 2026 13:15:54 -0700 Subject: [PATCH 3/7] 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 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 Date: Fri, 31 Jul 2026 13:53:40 -0700 Subject: [PATCH 4/7] 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 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 InsertionsByOffset; + struct InsertionInfo { + std::string Text; + unsigned Length = 0; + }; + llvm::DenseMap 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 Date: Mon, 17 Aug 2026 12:58:17 -0700 Subject: [PATCH 5/7] 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 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 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 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(); 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({"", Header("")}); + + 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 \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({"", Header("")}); + + 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 \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({"", Header("")}); + + 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 + +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 #include +#include +#include #include 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 insert(llvm::StringRef Header, - bool IsAngled, + std::optional insert(StringRef Header, bool IsAngled, IncludeDirective Directive) const; + /// Represents a single header directive to be inserted in a batch operation. + /// + /// Usage: + /// - HeaderToInsert("") -> inserts #include + /// (auto-detects angled) + /// - HeaderToInsert("\"foo.h\"") -> inserts #include "foo.h" + /// (auto-detects quoted) + /// - HeaderToInsert("", IncludeDirective::Import) -> inserts #import + /// + /// - 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 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 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> - IncludesByPriority; + std::unordered_map> 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 #include #include +#include #include #include #include #include +#include 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 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 Headers) const { + tooling::Replacements Result; + if (Headers.empty()) + return Result; + + std::vector 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 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 Date: Tue, 25 Aug 2026 14:51:15 -0700 Subject: [PATCH 6/7] 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({"", Header("")}); + 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 @@ -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 insert(StringRef Header, bool IsAngled, + std::optional 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 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 Headers) const; + tooling::Replacements insert(llvm::ArrayRef 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> IncludesByPriority; + std::unordered_map> + 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 Matches; + SmallVector 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 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; } } From 298bbdf7b98ca00ae5efc76f1b390109eaac4b99 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan Date: Tue, 25 Aug 2026 15:22:09 -0700 Subject: [PATCH 7/7] Some missed changes. - Reset Analysis back to original version. - Move tests to single change block. --- .../include-cleaner/lib/Analysis.cpp | 27 ++++--------- .../unittests/AnalysisTest.cpp | 38 +++++++++---------- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp index b1b055ee4728c..922a690dfc47c 100644 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -20,10 +20,8 @@ #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" @@ -33,6 +31,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" #include +#include #include #include @@ -168,23 +167,13 @@ std::string fixIncludes(const AnalysisResults &Results, const format::FormatStyle &Style) { assert(Style.isCpp() && "Only C++ style supports include insertions!"); tooling::Replacements R; - 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::SmallVector HeadersToInsert; - for (const auto &[Spelled, _] : Results.Missing) { - HeadersToInsert.emplace_back(Spelled, tooling::IncludeDirective::Include); - } - - for (const auto &Repl : HeaderIncludes.insert(HeadersToInsert)) { - cantFail(R.add(Repl)); - } + // 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))); + // "cleanup" actually turns the UINT_MAX replacements into concrete edits. 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 5d2643367c5bb..5df8792c93b2e 100644 --- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp @@ -706,25 +706,6 @@ TEST_F(WalkUsedTest, MacroConcat) { 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({"", Header("")}); - - 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 \nvoid bar();\n"); -} - TEST(FixIncludes, MainHeaderGrouping) { AnalysisResults Results; Results.Missing.push_back({"\"b.h\"", Header("\"b.h\"")}); @@ -783,5 +764,24 @@ TEST(FixIncludes, MultipleInsertionsSameOffset) { "#include \"a.h\"\n#include \"b.h\"\n"); } +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({"", Header("")}); + + 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 \nvoid bar();\n"); +} + } // namespace } // namespace clang::include_cleaner