Skip to content

Commit 6a98171

Browse files
authored
[clang-tidy] Speed up misc-header-include-cycle (#148757)
Performance optimization of misc-header-include-cycle based on clangd test on Sema.cpp. Check were slow due calls to SM.translateFile. Cost reduction (+-) from 11% to 3%.
1 parent 65dec99 commit 6a98171

File tree

2 files changed

+39
-40
lines changed

2 files changed

+39
-40
lines changed

clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp

Lines changed: 36 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
#include "clang/Lex/Preprocessor.h"
1414
#include "llvm/Support/Regex.h"
1515
#include <algorithm>
16-
#include <deque>
1716
#include <optional>
17+
#include <vector>
1818

1919
using namespace clang::ast_matchers;
2020

@@ -23,8 +23,8 @@ namespace clang::tidy::misc {
2323
namespace {
2424

2525
struct Include {
26-
FileID Id;
27-
llvm::StringRef Name;
26+
const FileEntry *File;
27+
StringRef Name;
2828
SourceLocation Loc;
2929
};
3030

@@ -50,31 +50,27 @@ class CyclicDependencyCallbacks : public PPCallbacks {
5050
if (Reason != EnterFile && Reason != ExitFile)
5151
return;
5252

53-
FileID Id = SM.getFileID(Loc);
53+
const FileID Id = SM.getFileID(Loc);
5454
if (Id.isInvalid())
5555
return;
5656

57+
const FileEntry *NewFile = SM.getFileEntryForID(Id);
58+
const FileEntry *PrevFile = SM.getFileEntryForID(PrevFID);
59+
5760
if (Reason == ExitFile) {
58-
if ((Files.size() > 1U) && (Files.back().Id == PrevFID) &&
59-
(Files[Files.size() - 2U].Id == Id))
61+
if ((Files.size() > 1U) && (Files.back().File == PrevFile) &&
62+
(Files[Files.size() - 2U].File == NewFile))
6063
Files.pop_back();
6164
return;
6265
}
6366

64-
if (!Files.empty() && Files.back().Id == Id)
67+
if (!Files.empty() && Files.back().File == NewFile)
6568
return;
6669

67-
std::optional<llvm::StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
68-
llvm::StringRef FileName =
69-
FilePath ? llvm::sys::path::filename(*FilePath) : llvm::StringRef();
70-
71-
if (!NextToEnter)
72-
NextToEnter = Include{Id, FileName, SourceLocation()};
73-
74-
assert(NextToEnter->Name == FileName);
75-
NextToEnter->Id = Id;
76-
Files.emplace_back(*NextToEnter);
77-
NextToEnter.reset();
70+
const std::optional<StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
71+
const StringRef FileName =
72+
FilePath ? llvm::sys::path::filename(*FilePath) : StringRef();
73+
Files.push_back({NewFile, FileName, std::exchange(NextToEnter, {})});
7874
}
7975

8076
void InclusionDirective(SourceLocation, const Token &, StringRef FilePath,
@@ -85,36 +81,26 @@ class CyclicDependencyCallbacks : public PPCallbacks {
8581
if (FileType != clang::SrcMgr::C_User)
8682
return;
8783

88-
llvm::StringRef FileName = llvm::sys::path::filename(FilePath);
89-
NextToEnter = {FileID(), FileName, Range.getBegin()};
84+
NextToEnter = Range.getBegin();
9085

9186
if (!File)
9287
return;
9388

94-
FileID Id = SM.translateFile(*File);
95-
if (Id.isInvalid())
96-
return;
97-
98-
checkForDoubleInclude(Id, FileName, Range.getBegin());
99-
}
100-
101-
void EndOfMainFile() override {
102-
if (!Files.empty() && Files.back().Id == SM.getMainFileID())
103-
Files.pop_back();
104-
105-
assert(Files.empty());
89+
checkForDoubleInclude(&File->getFileEntry(),
90+
llvm::sys::path::filename(FilePath),
91+
Range.getBegin());
10692
}
10793

108-
void checkForDoubleInclude(FileID Id, llvm::StringRef FileName,
94+
void checkForDoubleInclude(const FileEntry *File, StringRef FileName,
10995
SourceLocation Loc) {
110-
auto It =
111-
std::find_if(Files.rbegin(), Files.rend(),
112-
[&](const Include &Entry) { return Entry.Id == Id; });
96+
const auto It =
97+
llvm::find_if(llvm::reverse(Files),
98+
[&](const Include &Entry) { return Entry.File == File; });
11399
if (It == Files.rend())
114100
return;
115101

116-
const std::optional<StringRef> FilePath = SM.getNonBuiltinFilenameForID(Id);
117-
if (!FilePath || isFileIgnored(*FilePath))
102+
const StringRef FilePath = File->tryGetRealPathName();
103+
if (FilePath.empty() || isFileIgnored(FilePath))
118104
return;
119105

120106
if (It == Files.rbegin()) {
@@ -144,9 +130,19 @@ class CyclicDependencyCallbacks : public PPCallbacks {
144130
});
145131
}
146132

133+
#ifndef NDEBUG
134+
void EndOfMainFile() override {
135+
if (!Files.empty() &&
136+
Files.back().File == SM.getFileEntryForID(SM.getMainFileID()))
137+
Files.pop_back();
138+
139+
assert(Files.empty());
140+
}
141+
#endif
142+
147143
private:
148-
std::deque<Include> Files;
149-
std::optional<Include> NextToEnter;
144+
std::vector<Include> Files;
145+
SourceLocation NextToEnter;
150146
HeaderIncludeCycleCheck &Check;
151147
const SourceManager &SM;
152148
std::vector<llvm::Regex> IgnoredFilesRegexes;

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ Changes in existing checks
118118
<clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
119119
an additional matcher that generalizes the copy-and-swap idiom pattern
120120
detection.
121+
122+
- Improved :doc:`misc-header-include-cycle
123+
<clang-tidy/checks/misc/header-include-cycle>` check performance.
121124

122125
Removed checks
123126
^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)