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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
