[clang-tools-extra] [clang-tidy] Speed up `misc-header-include-cycle` (PR #148757)
Victor Chernyakin via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 15 11:24:08 PDT 2025
https://github.com/localspook updated https://github.com/llvm/llvm-project/pull/148757
>From 7cab196f263cc4d1787fe74ae49d4340fcb88624 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Sun, 13 Jul 2025 11:30:49 -0700
Subject: [PATCH 1/2] [clang-tidy] Speed up `misc-header-include-cycle`
---
.../misc/HeaderIncludeCycleCheck.cpp | 66 ++++++++-----------
1 file changed, 26 insertions(+), 40 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
index 21d443a1f34ff..1bd51c5902cde 100644
--- a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
@@ -13,8 +13,8 @@
#include "clang/Lex/Preprocessor.h"
#include "llvm/Support/Regex.h"
#include <algorithm>
-#include <deque>
#include <optional>
+#include <vector>
using namespace clang::ast_matchers;
@@ -23,8 +23,8 @@ namespace clang::tidy::misc {
namespace {
struct Include {
- FileID Id;
- llvm::StringRef Name;
+ const FileEntry *File;
+ StringRef Name;
SourceLocation Loc;
};
@@ -50,31 +50,27 @@ class CyclicDependencyCallbacks : public PPCallbacks {
if (Reason != EnterFile && Reason != ExitFile)
return;
- FileID Id = SM.getFileID(Loc);
+ const FileID Id = SM.getFileID(Loc);
if (Id.isInvalid())
return;
+ const FileEntry *NewFile = SM.getFileEntryForID(Id);
+ const FileEntry *PrevFile = SM.getFileEntryForID(PrevFID);
+
if (Reason == ExitFile) {
- if ((Files.size() > 1U) && (Files.back().Id == PrevFID) &&
- (Files[Files.size() - 2U].Id == Id))
+ if ((Files.size() > 1U) && (Files.back().File == PrevFile) &&
+ (Files[Files.size() - 2U].File == NewFile))
Files.pop_back();
return;
}
- if (!Files.empty() && Files.back().Id == Id)
+ if (!Files.empty() && Files.back().File == NewFile)
return;
- std::optional<llvm::StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
- llvm::StringRef FileName =
- FilePath ? llvm::sys::path::filename(*FilePath) : llvm::StringRef();
-
- if (!NextToEnter)
- NextToEnter = Include{Id, FileName, SourceLocation()};
-
- assert(NextToEnter->Name == FileName);
- NextToEnter->Id = Id;
- Files.emplace_back(*NextToEnter);
- NextToEnter.reset();
+ const std::optional<StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
+ const StringRef FileName =
+ FilePath ? llvm::sys::path::filename(*FilePath) : StringRef();
+ Files.push_back({NewFile, FileName, std::exchange(NextToEnter, {})});
}
void InclusionDirective(SourceLocation, const Token &, StringRef FilePath,
@@ -85,36 +81,26 @@ class CyclicDependencyCallbacks : public PPCallbacks {
if (FileType != clang::SrcMgr::C_User)
return;
- llvm::StringRef FileName = llvm::sys::path::filename(FilePath);
- NextToEnter = {FileID(), FileName, Range.getBegin()};
+ NextToEnter = Range.getBegin();
if (!File)
return;
- FileID Id = SM.translateFile(*File);
- if (Id.isInvalid())
- return;
-
- checkForDoubleInclude(Id, FileName, Range.getBegin());
+ checkForDoubleInclude(&File->getFileEntry(),
+ llvm::sys::path::filename(FilePath),
+ Range.getBegin());
}
- void EndOfMainFile() override {
- if (!Files.empty() && Files.back().Id == SM.getMainFileID())
- Files.pop_back();
-
- assert(Files.empty());
- }
-
- void checkForDoubleInclude(FileID Id, llvm::StringRef FileName,
+ void checkForDoubleInclude(const FileEntry *File, StringRef FileName,
SourceLocation Loc) {
- auto It =
- std::find_if(Files.rbegin(), Files.rend(),
- [&](const Include &Entry) { return Entry.Id == Id; });
+ const auto It =
+ llvm::find_if(llvm::reverse(Files),
+ [&](const Include &Entry) { return Entry.File == File; });
if (It == Files.rend())
return;
- const std::optional<StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
- if (!FilePath || isFileIgnored(*FilePath))
+ const StringRef FilePath = File->tryGetRealPathName();
+ if (FilePath.empty() || isFileIgnored(FilePath))
return;
if (It == Files.rbegin()) {
@@ -145,8 +131,8 @@ class CyclicDependencyCallbacks : public PPCallbacks {
}
private:
- std::deque<Include> Files;
- std::optional<Include> NextToEnter;
+ std::vector<Include> Files;
+ SourceLocation NextToEnter;
HeaderIncludeCycleCheck &Check;
const SourceManager &SM;
std::vector<llvm::Regex> IgnoredFilesRegexes;
>From 477eef1d60387b66eb477822827238e537178ea1 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Tue, 15 Jul 2025 11:23:13 -0700
Subject: [PATCH 2/2] Add release notes, restore `EndOfMainFile` override
---
.../clang-tidy/misc/HeaderIncludeCycleCheck.cpp | 10 ++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
2 files changed, 13 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
index 1bd51c5902cde..1f6ceda9f5b9e 100644
--- a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
@@ -130,6 +130,16 @@ class CyclicDependencyCallbacks : public PPCallbacks {
});
}
+#ifndef NDEBUG
+ void EndOfMainFile() override {
+ if (!Files.empty() &&
+ Files.back().File == SM.getFileEntryForID(SM.getMainFileID()))
+ Files.pop_back();
+
+ assert(Files.empty());
+ }
+#endif
+
private:
std::vector<Include> Files;
SourceLocation NextToEnter;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ad869265a2db5..fb7f22f110740 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -257,6 +257,9 @@ Changes in existing checks
`AnalyzePointers` option and fixing false positives when using const array
type.
+- Improved :doc:`misc-header-include-cycle
+ <clang-tidy/checks/misc/header-include-cycle>` check performance.
+
- Improved :doc:`misc-include-cleaner
<clang-tidy/checks/misc/include-cleaner>` check by adding the options
`UnusedIncludes` and `MissingIncludes`, which specify whether the check should
More information about the cfe-commits
mailing list