Author: Victor Chernyakin Date: 2025-07-23T20:14:29+02:00 New Revision: 6a9817113838a3f87e803ac71aab46b3ccf24686
URL: https://github.com/llvm/llvm-project/commit/6a9817113838a3f87e803ac71aab46b3ccf24686 DIFF: https://github.com/llvm/llvm-project/commit/6a9817113838a3f87e803ac71aab46b3ccf24686.diff LOG: [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%. Added: Modified: clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp index 21d443a1f34ff..1f6ceda9f5b9e 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()); - } - - void EndOfMainFile() override { - if (!Files.empty() && Files.back().Id == SM.getMainFileID()) - Files.pop_back(); - - assert(Files.empty()); + checkForDoubleInclude(&File->getFileEntry(), + llvm::sys::path::filename(FilePath), + Range.getBegin()); } - 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()) { @@ -144,9 +130,19 @@ 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::deque<Include> Files; - std::optional<Include> NextToEnter; + std::vector<Include> Files; + SourceLocation NextToEnter; HeaderIncludeCycleCheck &Check; const SourceManager &SM; std::vector<llvm::Regex> IgnoredFilesRegexes; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index bd2d6eb55b29a..c5f756aeee6eb 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -118,6 +118,9 @@ Changes in existing checks <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding an additional matcher that generalizes the copy-and-swap idiom pattern detection. + +- Improved :doc:`misc-header-include-cycle + <clang-tidy/checks/misc/header-include-cycle>` check performance. Removed checks ^^^^^^^^^^^^^^ _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
