diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -19,6 +19,7 @@ ConstCorrectnessCheck.cpp DefinitionsInHeadersCheck.cpp ConfusableIdentifierCheck.cpp + HeaderIncludeCycleCheck.cpp MiscTidyModule.cpp MisleadingBidirectional.cpp MisleadingIdentifier.cpp diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.h @@ -0,0 +1,30 @@ +//===--- HeaderIncludeCycleCheck.h - clang-tidy -----------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERINCLUDECYCLECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERINCLUDECYCLECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::misc { + +/// Check detects cyclic #include dependencies between user-defined headers. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/misc/header-include-cycle.html +class HeaderIncludeCycleCheck : public ClangTidyCheck { +public: + HeaderIncludeCycleCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; +}; + +} // namespace clang::tidy::misc + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_HEADERINCLUDECYCLECHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp @@ -0,0 +1,120 @@ +//===--- HeaderIncludeCycleCheck.cpp - clang-tidy -------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "HeaderIncludeCycleCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/SmallVector.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::misc { + +namespace { + +class CyclicDependencyCallbacks : public PPCallbacks { +public: + CyclicDependencyCallbacks(HeaderIncludeCycleCheck &Check, + const SourceManager &SM) + : Check(Check), SM(SM) { + Files.emplace_back(SM.getMainFileID()); + } + + void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override { + if (FileType != clang::SrcMgr::C_User) + return; + + if (Reason != EnterFile && Reason != ExitFile) + return; + + FileID Id = SM.getFileID(Loc); + if (Id.isInvalid()) + return; + + if (Reason == EnterFile) { + if (Files.empty() || Files.back() != Id) { + Files.emplace_back(Id); + } + } else if ((Files.size() > 1U) && (Files.back() == PrevFID) && + (Files[Files.size() - 2U] == Id)) + Files.pop_back(); + } + + void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok, + SrcMgr::CharacteristicKind FileType) override { + if (FileType != clang::SrcMgr::C_User) + return; + + FileID Id = SM.translateFile(SkippedFile); + if (Id.isInvalid()) + return; + + auto It = std::find(Files.begin(), Files.end(), Id); + if (It == Files.end()) + return; + SmallVector IncludePath; + + while (It != Files.end()) { + std::optional FilePath = + SM.getNonBuiltinFilenameForID(*It); + ++It; + if (not FilePath) + continue; + IncludePath.emplace_back(FilePath->rsplit("/").second); + } + + std::optional FilePath = SM.getNonBuiltinFilenameForID(Id); + if (not FilePath) + return; + + IncludePath.emplace_back(FilePath->rsplit("/").second); + reportDiagnostic(FilenameTok.getLocation(), IncludePath); + } + + void EndOfMainFile() override { assert(Files.size() == 1U); } + + void reportDiagnostic(const SourceLocation &Loc, + const SmallVector &History) { + if (History.size() == 2U) { + Check.diag(Loc, "direct self-inclusion of header file '%0'") + << History.back(); + return; + } + + std::string IncludePath; + for (const llvm::StringRef &Name : History) { + IncludePath += '\''; + IncludePath += Name; + IncludePath += "' -> "; + } + + Check.diag(Loc, "circular header file dependency detected while including " + "'%0', please check the include path: %1") + << History.back() << llvm::StringRef(IncludePath).rsplit(" -> ").first; + } + +private: + SmallVector Files; + HeaderIncludeCycleCheck &Check; + const SourceManager &SM; +}; + +} // namespace + +void HeaderIncludeCycleCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks(std::make_unique(*this, SM)); +} + +} // namespace clang::tidy::misc diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -12,6 +12,7 @@ #include "ConfusableIdentifierCheck.h" #include "ConstCorrectnessCheck.h" #include "DefinitionsInHeadersCheck.h" +#include "HeaderIncludeCycleCheck.h" #include "MisleadingBidirectional.h" #include "MisleadingIdentifier.h" #include "MisplacedConstCheck.h" @@ -41,6 +42,8 @@ "misc-const-correctness"); CheckFactories.registerCheck( "misc-definitions-in-headers"); + CheckFactories.registerCheck( + "misc-header-include-cycle"); CheckFactories.registerCheck( "misc-misleading-bidirectional"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -120,6 +120,11 @@ Checks that all implicit and explicit inline functions in header files are tagged with the ``LIBC_INLINE`` macro. +- New :doc:`misc-header-include-cycle + ` check. + + Check detects cyclic ``#include`` dependencies between user-defined headers. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -249,6 +249,7 @@ `misc-confusable-identifiers `_, `misc-const-correctness `_, "Yes" `misc-definitions-in-headers `_, "Yes" + `misc-header-include-cycle `_, `misc-misleading-bidirectional `_, `misc-misleading-identifier `_, `misc-misplaced-const `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/header-include-cycle.rst @@ -0,0 +1,63 @@ +.. title:: clang-tidy - misc-header-include-cycle + +misc-header-include-cycle +========================= + +Check detects cyclic ``#include`` dependencies between user-defined headers. + +.. code-block:: c++ + + // Header A.hpp + #pragma once + #include "B.hpp" + + // Header B.hpp + #pragma once + #include "C.hpp" + + // Header C.hpp + #pragma once + #include "A.hpp" + + // Include chain: A->B->C->A + +Header files are a crucial part of many C++ programs, as they provide a way to +organize declarations and definitions that are shared across multiple source +files. However, header files can also create problems when they become entangled +in complex dependency cycles. Such cycles can cause issues with compilation +times, unnecessary rebuilds, and make it harder to understand the overall +structure of the code. + +To address these issues, this check has been developed. This check is designed +to detect cyclic dependencies between header files, also known as +"include cycles". An include cycle occurs when a header file `A` includes a +header file `B`, and header file `B` (or any later included header file in the +chain) includes back header file `A`, leading to a circular dependency cycle. + +This check operates at the preprocessor level and analyzes user-defined headers +and their dependencies. It focuses specifically on detecting include cycles, +and ignores other types or function dependencies. This allows it to provide a +specialized analysis that is focused on identifying and preventing issues +related to header file organization. + +The benefits of using this check are numerous. By detecting include cycles early +in the development process, developers can identify and resolve these issues +before they become more difficult and time-consuming to fix. This can lead to +faster compile times, improved code quality, and a more maintainable codebase +overall. Additionally, by ensuring that header files are organized in a way that +avoids cyclic dependencies, developers can make their code easier to understand +and modify over time. + +It's worth noting that this tool only analyzes user-defined headers and their +dependencies, excluding system includes such as standard library headers and +third-party library headers. System includes are usually well-designed and free +of include cycles, and ignoring them helps to focus on potential issues within +the project's own codebase. This limitation doesn't diminish the tool's ability +to detect ``#include`` cycles within the analyzed code. As with any tool, +developers should use their judgment when evaluating the warnings produced by +the check and be prepared to make exceptions or modifications to their code as +needed. + +This check has no options to configure. + +**Catch header cycles before they catch you - Try this Clang-tidy check today!** diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.first.hpp @@ -0,0 +1,2 @@ +#pragma once +#include "header-include-cycle.second.hpp" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.fourth.hpp @@ -0,0 +1,2 @@ +#pragma once +#include "header-include-cycle.first.hpp" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.second.hpp @@ -0,0 +1,2 @@ +#pragma once +#include "header-include-cycle.third.hpp" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.self.hpp @@ -0,0 +1,2 @@ +#pragma once +#include "header-include-cycle.self.hpp" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/header-include-cycle.third.hpp @@ -0,0 +1,2 @@ +#pragma once +#include "header-include-cycle.fourth.hpp" diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/header-include-cycle.cpp @@ -0,0 +1,11 @@ +// RUN: rm -rf %T/misc-header-include-cycle-headers +// RUN: mkdir %T/misc-header-include-cycle-headers +// RUN: cp -r %S/Inputs/header-include-cycle* %T/misc-header-include-cycle-headers/ +// RUN: clang-tidy %s -checks='-*,misc-header-include-cycle' -header-filter=.* -- -I%T/misc-header-include-cycle-headers | FileCheck %s -check-prefix=CHECK-MESSAGES -implicit-check-not="{{warning|error}}:" +// RUN: rm -rf %T/misc-header-include-cycle-headers + +#include +// CHECK-MESSAGES: header-include-cycle.fourth.hpp:2:10: warning: circular header file dependency detected while including 'header-include-cycle.first.hpp', please check the include path: 'header-include-cycle.first.hpp' -> 'header-include-cycle.second.hpp' -> 'header-include-cycle.third.hpp' -> 'header-include-cycle.fourth.hpp' -> 'header-include-cycle.first.hpp' [misc-header-include-cycle] + +#include +// CHECK-MESSAGES: header-include-cycle.self.hpp:2:10: warning: direct self-inclusion of header file 'header-include-cycle.self.hpp' [misc-header-include-cycle]