[llvm-branch-commits] [clang-tools-extra] [include-cleaner] add fragment dependency comments (PR #196766)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat May 9 15:45:56 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Daniil Dudkin (unterumarmung)
<details>
<summary>Changes</summary>
---
Patch is 25.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/196766.diff
10 Files Affected:
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-1)
- (modified) clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h (+44-1)
- (modified) clang-tools-extra/include-cleaner/lib/Analysis.cpp (+146-5)
- (added) clang-tools-extra/include-cleaner/test/fragments-block-comment.cpp (+6)
- (added) clang-tools-extra/include-cleaner/test/fragments-empty-comment.cpp (+6)
- (modified) clang-tools-extra/include-cleaner/test/fragments-multi.cpp (+3-1)
- (modified) clang-tools-extra/include-cleaner/test/fragments-spelled-path.cpp (+3-1)
- (modified) clang-tools-extra/include-cleaner/test/fragments.cpp (+11-1)
- (modified) clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp (+38-5)
- (modified) clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp (+192)
``````````diff
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 22f760f616473..c508cd1bce510 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -654,7 +654,9 @@ Improvements to clang-include-cleaner
-------------------------------------
- Added :program:`clang-include-cleaner` support for treating matching direct
- includes as fragments of the main file with ``--fragment-headers``.
+ includes as fragments of the main file with ``--fragment-headers`` and for
+ optionally annotating preserved includes with
+ ``--fragment-dependency-comment-format``.
Improvements to clang-include-fixer
-----------------------------------
diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 1d28d87c025ca..268936cebd0d5 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -16,11 +16,13 @@
#include "clang/Format/Format.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <functional>
+#include <optional>
#include <string>
#include <vector>
@@ -31,7 +33,6 @@ class Decl;
class FileEntry;
class HeaderSearch;
namespace tooling {
-class Replacements;
struct IncludeStyle;
} // namespace tooling
namespace include_cleaner {
@@ -119,12 +120,54 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
const Preprocessor &PP,
const AnalysisOptions &Options = {});
+enum class FragmentDependencyCommentStatus {
+ CanInsert,
+ AlreadyPresent,
+ ConflictingComment,
+};
+
+/// Planned comment state for an include kept alive by fragments.
+struct FragmentDependencyComment {
+ const Include *Preserved = nullptr;
+ llvm::SmallVector<const Include *> Fragments;
+ std::string Text;
+ FragmentDependencyCommentStatus Status =
+ FragmentDependencyCommentStatus::CanInsert;
+ std::optional<tooling::Replacement> Replacement;
+};
+
+/// Replacements computed from include-cleaner findings.
+struct IncludeFixes {
+ tooling::Replacements Replacements;
+ std::vector<FragmentDependencyComment> FragmentComments;
+};
+
+/// Options for turning analysis results into source edits.
+struct FixIncludesOptions {
+ /// Raw trailing comment text without the leading //.
+ ///
+ /// When it contains `{0}`, that placeholder is replaced with a comma-
+ /// separated list of direct fragment include spellings that keep the include
+ /// alive.
+ llvm::StringRef FragmentDependencyCommentFormat;
+};
+
+/// Computes replacements to apply include-cleaner findings to the main file.
+IncludeFixes computeIncludeFixes(const AnalysisResults &Results,
+ llvm::StringRef FileName, llvm::StringRef Code,
+ const format::FormatStyle &IncludeStyle,
+ const FixIncludesOptions &Options = {});
+
/// Removes unused includes and inserts missing ones in the main file.
/// Returns the modified main-file code.
/// The FormatStyle must be C++ or ObjC (to support include ordering).
std::string fixIncludes(const AnalysisResults &Results,
llvm::StringRef FileName, llvm::StringRef Code,
const format::FormatStyle &IncludeStyle);
+std::string fixIncludes(const AnalysisResults &Results,
+ llvm::StringRef FileName, llvm::StringRef Code,
+ const format::FormatStyle &IncludeStyle,
+ const FixIncludesOptions &Options);
/// Gets all the providers for a symbol by traversing each location.
/// Returned headers are sorted by relevance, first element is the most
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index 321b67a5787d3..34608af9b1189 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -379,19 +379,160 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
return Results;
}
-std::string fixIncludes(const AnalysisResults &Results,
- llvm::StringRef FileName, llvm::StringRef Code,
- const format::FormatStyle &Style) {
+namespace {
+
+// The fix planner only receives FileName and Code, so comment edits map
+// Include::Line back to offsets in that text.
+class LineIndex {
+public:
+ explicit LineIndex(llvm::StringRef Code) : Code(Code) {
+ Starts.push_back(0);
+ for (unsigned I = 0; I < Code.size(); ++I) {
+ if (Code[I] == '\n')
+ Starts.push_back(I + 1);
+ }
+ Starts.push_back(Code.size());
+ }
+
+ llvm::StringRef line(unsigned OneBasedLine) const {
+ auto [Start, End] = bounds(OneBasedLine);
+ return Code.slice(Start, End);
+ }
+
+ unsigned trimmedLineEnd(unsigned OneBasedLine) const {
+ auto [Start, End] = bounds(OneBasedLine);
+ while (End > Start && (Code[End - 1] == ' ' || Code[End - 1] == '\t'))
+ --End;
+ return End;
+ }
+
+private:
+ std::pair<unsigned, unsigned> bounds(unsigned OneBasedLine) const {
+ if (OneBasedLine == 0 || OneBasedLine >= Starts.size())
+ return {Code.size(), Code.size()};
+ unsigned Start = Starts[OneBasedLine - 1];
+ unsigned End =
+ Starts[OneBasedLine] == 0 ? Code.size() : Starts[OneBasedLine] - 1;
+ if (End > Start && Code[End - 1] == '\r')
+ --End;
+ return {Start, End};
+ }
+
+ llvm::StringRef Code;
+ llvm::SmallVector<unsigned> Starts;
+};
+
+std::string formatFragmentDependencyComment(
+ llvm::StringRef Format, llvm::ArrayRef<const Include *> FragmentIncludes) {
+ if (Format.empty())
+ return {};
+
+ std::string FragmentList;
+ for (const Include *Fragment : FragmentIncludes) {
+ if (!FragmentList.empty())
+ FragmentList += ", ";
+ FragmentList += Fragment->quote();
+ }
+
+ std::string Result;
+ llvm::StringRef Remaining = Format;
+ while (true) {
+ auto Pos = Remaining.find("{0}");
+ if (Pos == llvm::StringRef::npos) {
+ Result += Remaining.str();
+ return Result;
+ }
+ Result += Remaining.take_front(Pos).str();
+ Result += FragmentList;
+ Remaining = Remaining.drop_front(Pos + 3);
+ }
+}
+
+FragmentDependencyComment inspectFragmentDependencyComment(
+ const FragmentDependency &Dependency, llvm::StringRef FileName,
+ const LineIndex &Lines, llvm::StringRef CommentFormat) {
+ FragmentDependencyComment Comment{
+ Dependency.Preserved, Dependency.Fragments,
+ formatFragmentDependencyComment(CommentFormat, Dependency.Fragments),
+ FragmentDependencyCommentStatus::CanInsert, std::nullopt};
+ if (Comment.Text.empty())
+ return Comment;
+
+ llvm::StringRef Line = Lines.line(Dependency.Preserved->Line);
+ size_t IncludePos = Line.find(Dependency.Preserved->quote());
+ if (IncludePos == llvm::StringRef::npos) {
+ Comment.Status = FragmentDependencyCommentStatus::ConflictingComment;
+ return Comment;
+ }
+
+ llvm::StringRef Tail =
+ Line.drop_front(IncludePos + Dependency.Preserved->quote().size());
+ llvm::StringRef TrimmedTail = Tail.ltrim(" \t");
+ if (TrimmedTail.empty()) {
+ Comment.Status = FragmentDependencyCommentStatus::CanInsert;
+ Comment.Replacement = tooling::Replacement(
+ FileName, Lines.trimmedLineEnd(Dependency.Preserved->Line), 0,
+ " // " + Comment.Text);
+ return Comment;
+ }
+
+ if (TrimmedTail.starts_with("//")) {
+ llvm::StringRef Existing = TrimmedTail.drop_front(2).trim();
+ Comment.Status = Existing == Comment.Text
+ ? FragmentDependencyCommentStatus::AlreadyPresent
+ : FragmentDependencyCommentStatus::ConflictingComment;
+ return Comment;
+ }
+
+ Comment.Status = FragmentDependencyCommentStatus::ConflictingComment;
+ return Comment;
+}
+
+} // namespace
+
+IncludeFixes computeIncludeFixes(const AnalysisResults &Results,
+ llvm::StringRef FileName, llvm::StringRef Code,
+ const format::FormatStyle &Style,
+ const FixIncludesOptions &Options) {
assert(Style.isCpp() && "Only C++ style supports include insertions!");
- tooling::Replacements R;
+ IncludeFixes Fixes;
+ tooling::Replacements &R = Fixes.Replacements;
// 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 (const MissingInclude &Missing : Results.MissingIncludes)
cantFail(R.add(tooling::Replacement(FileName, UINT_MAX, 0,
"#include " + Missing.Spelling)));
+
+ if (Options.FragmentDependencyCommentFormat.empty())
+ return Fixes;
+
+ LineIndex Lines(Code);
+ for (const FragmentDependency &Dependency : Results.FragmentDependencies) {
+ FragmentDependencyComment Comment = inspectFragmentDependencyComment(
+ Dependency, FileName, Lines, Options.FragmentDependencyCommentFormat);
+ if (Comment.Replacement)
+ cantFail(R.add(*Comment.Replacement));
+ Fixes.FragmentComments.push_back(std::move(Comment));
+ }
+ return Fixes;
+}
+
+std::string fixIncludes(const AnalysisResults &Results,
+ llvm::StringRef FileName, llvm::StringRef Code,
+ const format::FormatStyle &Style) {
+ return fixIncludes(Results, FileName, Code, Style, {});
+}
+
+std::string fixIncludes(const AnalysisResults &Results,
+ llvm::StringRef FileName, llvm::StringRef Code,
+ const format::FormatStyle &Style,
+ const FixIncludesOptions &Options) {
+ IncludeFixes Fixes =
+ computeIncludeFixes(Results, FileName, Code, Style, Options);
// "cleanup" actually turns the UINT_MAX replacements into concrete edits.
- auto Positioned = cantFail(format::cleanupAroundReplacements(Code, R, Style));
+ auto Positioned = cantFail(
+ format::cleanupAroundReplacements(Code, Fixes.Replacements, Style));
return cantFail(tooling::applyAllReplacements(Code, Positioned));
}
diff --git a/clang-tools-extra/include-cleaner/test/fragments-block-comment.cpp b/clang-tools-extra/include-cleaner/test/fragments-block-comment.cpp
new file mode 100644
index 0000000000000..fc2a004fbe324
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/test/fragments-block-comment.cpp
@@ -0,0 +1,6 @@
+#include <vector> /* keep me */
+#include "gen.inc"
+
+Gen G;
+
+// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' --fragment-dependency-comment-format='needed by {0}' -- -I%S/Inputs/ | count 0
diff --git a/clang-tools-extra/include-cleaner/test/fragments-empty-comment.cpp b/clang-tools-extra/include-cleaner/test/fragments-empty-comment.cpp
new file mode 100644
index 0000000000000..0e7ee5615e7d3
--- /dev/null
+++ b/clang-tools-extra/include-cleaner/test/fragments-empty-comment.cpp
@@ -0,0 +1,6 @@
+#include <vector>
+#include "gen.inc"
+
+Gen G;
+
+// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' -- -I%S/Inputs/ | count 0
diff --git a/clang-tools-extra/include-cleaner/test/fragments-multi.cpp b/clang-tools-extra/include-cleaner/test/fragments-multi.cpp
index 5aa1814df7326..89cfd96c4350f 100644
--- a/clang-tools-extra/include-cleaner/test/fragments-multi.cpp
+++ b/clang-tools-extra/include-cleaner/test/fragments-multi.cpp
@@ -5,4 +5,6 @@
A AValue;
B BValue;
-// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' -- -I%S/Inputs/ | count 0
+// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' --fragment-dependency-comment-format='needed by {0}' -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGES %s
+// CHANGES: ~ <vector> @Line:1 // needed by "a.inc", "b.inc"
+// CHANGES-NOT: - <vector>
diff --git a/clang-tools-extra/include-cleaner/test/fragments-spelled-path.cpp b/clang-tools-extra/include-cleaner/test/fragments-spelled-path.cpp
index 82d0d27c34df9..45174240ab498 100644
--- a/clang-tools-extra/include-cleaner/test/fragments-spelled-path.cpp
+++ b/clang-tools-extra/include-cleaner/test/fragments-spelled-path.cpp
@@ -3,4 +3,6 @@
SpelledGen G;
-// RUN: clang-include-cleaner -print=changes %s --fragment-headers='generated/gen\.inc' -- -I%S/Inputs/ | count 0
+// RUN: clang-include-cleaner -print=changes %s --fragment-headers='generated/gen\.inc' --fragment-dependency-comment-format='needed by {0}' -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGES %s
+// CHANGES: ~ <vector> @Line:1 // needed by "generated/gen.inc"
+// CHANGES-NOT: - <vector>
diff --git a/clang-tools-extra/include-cleaner/test/fragments.cpp b/clang-tools-extra/include-cleaner/test/fragments.cpp
index 0e7ee5615e7d3..159fc5736984c 100644
--- a/clang-tools-extra/include-cleaner/test/fragments.cpp
+++ b/clang-tools-extra/include-cleaner/test/fragments.cpp
@@ -3,4 +3,14 @@
Gen G;
-// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' -- -I%S/Inputs/ | count 0
+// RUN: clang-include-cleaner -print=changes %s --fragment-headers='.*\.inc$' --fragment-dependency-comment-format='needed by {0}' -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGES %s
+// CHANGES: ~ <vector> @Line:1 // needed by "gen.inc"
+// CHANGES-NOT: - <vector>
+
+// RUN: clang-include-cleaner -print %s --fragment-headers='.*\.inc$' --fragment-dependency-comment-format='needed by {0}' -- -I%S/Inputs/ | FileCheck --check-prefix=PRINT %s
+// PRINT: #include <vector> // needed by "gen.inc"
+
+// RUN: cp %s %t.cpp
+// RUN: clang-include-cleaner -edit %t.cpp --fragment-headers='.*\.inc$' --fragment-dependency-comment-format='IWYU pragma: keep' -- -I%S/Inputs/
+// RUN: FileCheck --check-prefix=EDIT %s < %t.cpp
+// EDIT: #include <vector> // IWYU pragma: keep
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index fbc14be848b5f..675e59a37c601 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -88,6 +88,15 @@ cl::opt<std::string> FragmentHeaders{
cl::cat(IncludeCleaner),
};
+cl::opt<std::string> FragmentDependencyCommentFormat{
+ "fragment-dependency-comment-format",
+ cl::desc("Append this trailing comment to includes that are used only by "
+ "fragment headers. Use {0} for the matched fragment include "
+ "spellings."),
+ cl::init(""),
+ cl::cat(IncludeCleaner),
+};
+
enum class PrintStyle { Changes, Final };
cl::opt<PrintStyle> Print{
"print",
@@ -147,9 +156,12 @@ class Action : public clang::ASTFrontendAction {
public:
Action(std::function<bool(const Header &)> HeaderFilter,
std::function<bool(llvm::StringRef)> FragmentHeaderFilter,
+ std::string FragmentDependencyCommentFormat,
llvm::StringMap<std::string> &EditedFiles)
: HeaderFilter(std::move(HeaderFilter)),
FragmentHeaderFilter(std::move(FragmentHeaderFilter)),
+ FragmentDependencyCommentFormat(
+ std::move(FragmentDependencyCommentFormat)),
EditedFiles(EditedFiles) {}
private:
@@ -158,6 +170,7 @@ class Action : public clang::ASTFrontendAction {
PragmaIncludes PI;
std::function<bool(const Header &)> HeaderFilter;
std::function<bool(llvm::StringRef)> FragmentHeaderFilter;
+ std::string FragmentDependencyCommentFormat;
llvm::StringMap<std::string> &EditedFiles;
bool BeginInvocation(CompilerInstance &CI) override {
@@ -237,7 +250,14 @@ class Action : public clang::ASTFrontendAction {
Results.MissingIncludes.clear();
if (!Remove || DisableRemove)
Results.Unused.clear();
- std::string Final = fixIncludes(Results, AbsPath, Code, getStyle(AbsPath));
+ format::FormatStyle Style = getStyle(AbsPath);
+ FixIncludesOptions FixOptions{FragmentDependencyCommentFormat};
+ IncludeFixes Fixes =
+ computeIncludeFixes(Results, AbsPath, Code, Style, FixOptions);
+ auto Positioned = cantFail(
+ format::cleanupAroundReplacements(Code, Fixes.Replacements, Style));
+ std::string Final =
+ cantFail(tooling::applyAllReplacements(Code, Positioned));
if (Print.getNumOccurrences()) {
switch (Print) {
@@ -246,6 +266,13 @@ class Action : public clang::ASTFrontendAction {
llvm::outs() << "- " << I->quote() << " @Line:" << I->Line << "\n";
for (const MissingInclude &I : Results.MissingIncludes)
llvm::outs() << "+ " << I.Spelling << "\n";
+ for (const auto &Comment : Fixes.FragmentComments) {
+ if (!Comment.Replacement)
+ continue;
+ llvm::outs() << "~ " << Comment.Preserved->quote()
+ << " @Line:" << Comment.Preserved->Line << " // "
+ << Comment.Text << "\n";
+ }
break;
case PrintStyle::Final:
llvm::outs() << Final;
@@ -253,7 +280,7 @@ class Action : public clang::ASTFrontendAction {
}
}
- if (!Results.MissingIncludes.empty() || !Results.Unused.empty())
+ if (!Fixes.Replacements.empty())
EditedFiles.try_emplace(AbsPath, Final);
}
@@ -274,12 +301,16 @@ class Action : public clang::ASTFrontendAction {
class ActionFactory : public tooling::FrontendActionFactory {
public:
ActionFactory(std::function<bool(const Header &)> HeaderFilter,
- std::function<bool(llvm::StringRef)> FragmentHeaderFilter)
+ std::function<bool(llvm::StringRef)> FragmentHeaderFilter,
+ std::string FragmentDependencyCommentFormat)
: HeaderFilter(std::move(HeaderFilter)),
- FragmentHeaderFilter(std::move(FragmentHeaderFilter)) {}
+ FragmentHeaderFilter(std::move(FragmentHeaderFilter)),
+ FragmentDependencyCommentFormat(
+ std::move(FragmentDependencyCommentFormat)) {}
std::unique_ptr<clang::FrontendAction> create() override {
return std::make_unique<Action>(HeaderFilter, FragmentHeaderFilter,
+ FragmentDependencyCommentFormat,
EditedFiles);
}
@@ -290,6 +321,7 @@ class ActionFactory : public tooling::FrontendActionFactory {
private:
std::function<bool(const Header &)> HeaderFilter;
std::function<bool(llvm::StringRef)> FragmentHeaderFilter;
+ std::string FragmentDependencyCommentFormat;
// Map from file name to final code with the include edits applied.
llvm::StringMap<std::string> EditedFiles;
};
@@ -431,7 +463,8 @@ int main(int argc, const char **argv) {
auto FragmentHeaderFilter = fragmentHeaderFilter();
if (!HeaderFilter || !FragmentHeaderFilter)
return 1; // error already reported.
- ActionFactory Factory(HeaderFilter, FragmentHeaderFilter);
+ ActionFactory Factory(HeaderFilter, FragmentHeaderFilter,
+ FragmentDependencyCommentFormat);
auto ErrorCode = Tool.run(&Factory);
if (Edit) {
for (const auto &NameAndContent : Factory.editedFiles()) {
diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index 0c68fb58032fb..64a4fb133bf22 100644
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -812,6 +812,198 @@ TEST(FixIncludes, Basic) {
#include "a.h")cpp");
}
+TEST(FixIncludes, FragmentDependencyComments) {
+ llvm::StringRef Code = R"cpp(#include "a.h"
+#include "gen.inc"
+#include "existing.h" // needed by "gen.inc"
+#include "conflict.h" // keep me
+)cpp";
+
+ Includes Inc;
+ Include A;
+ A.Spelled = "a.h";
+ A.Line = 1;
+ Inc.add(A);
+ Include Gen;
+ Gen.Spelled = "gen.inc";
+ Gen.Line = 2;
+ Inc.add(Gen);
+ Include Existing;
+ Existing.Spelled = "existing.h";
+ Existing.Line = 3;
+ Inc.add(Existing);
+ Include Conflict;
+ Conflict.Spelled = "conflict.h";
+ Conflict.Line = 4;
+ Inc.add(Conflict);
...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/196766
More information about the llvm-branch-commits
mailing list