diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp index a0e7ac19ab2d5..bf0d9a7598679 100644 --- a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp @@ -107,7 +107,7 @@ class CyclicDependencyCallbacks : public PPCallbacks { Check.diag(Loc, "direct self-inclusion of header file '%0'") << FileName; return; } - + Check.diag(Loc, "circular header file dependency detected while including " "'%0', please check the include path") << FileName; diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp index cc9ae471a926d..294202e430d84 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp @@ -7,11 +7,16 @@ //===----------------------------------------------------------------------===// #include "DuplicateIncludeCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/Regex.h" #include +#include namespace clang::tidy::readability { @@ -33,11 +38,8 @@ using FileList = SmallVector; class DuplicateIncludeCallbacks : public PPCallbacks { public: DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check, - const SourceManager &SM) - : Check(Check), SM(SM) { - // The main file doesn't participate in the FileChanged notification. - Files.emplace_back(); - } + const SourceManager &SM, + const std::vector &AllowedStrings); void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -62,10 +64,25 @@ class DuplicateIncludeCallbacks : public PPCallbacks { SmallVector Files; DuplicateIncludeCheck &Check; const SourceManager &SM; + std::vector AllowedDuplicateRegex; + + bool IsAllowedDuplicateInclude(StringRef TokenName); }; } // namespace +DuplicateIncludeCallbacks::DuplicateIncludeCallbacks( + DuplicateIncludeCheck &Check, const SourceManager &SM, + const std::vector &AllowedStrings) + : Check(Check), SM(SM) { + // The main file doesn't participate in the FileChanged notification. + Files.emplace_back(); + AllowedDuplicateRegex.reserve(AllowedStrings.size()); + for (const std::string &str : AllowedStrings) { + AllowedDuplicateRegex.emplace_back(str); + } +} + void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -85,6 +102,13 @@ void DuplicateIncludeCallbacks::InclusionDirective( if (FilenameRange.getBegin().isMacroID() || FilenameRange.getEnd().isMacroID()) return; + + // if duplicate allowed, record and return + if (IsAllowedDuplicateInclude(FileName)) { + Files.back().push_back(FileName); + return; + } + if (llvm::is_contained(Files.back(), FileName)) { // We want to delete the entire line, so make sure that [Start,End] covers // everything. @@ -109,9 +133,41 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok, Files.back().clear(); } +bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude(StringRef FileName) { + // try to match with each regex + for (const llvm::Regex ® : AllowedDuplicateRegex) { + if (reg.match(FileName)) + return true; + } + return false; +} +} // namespace clang::tidy::readability + +namespace clang::tidy::readability { +DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) { + std::string Raw = Options.get("IgnoreHeaders", "").str(); + if (!Raw.empty()) { + SmallVector StringParts; + StringRef(Raw).split(StringParts, ';', -1, false); + + for (StringRef Part : StringParts) { + Part = Part.trim(); + if (!Part.empty()) + AllowedDuplicateIncludes.push_back(Part.str()); + } + } +} + void DuplicateIncludeCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique(*this, SM)); + PP->addPPCallbacks(std::make_unique( + *this, SM, AllowedDuplicateIncludes)); } +void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoreHeaders", + llvm::join(AllowedDuplicateIncludes, ";")); +} } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h index ca3679108e60b..784a92800ed57 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h @@ -10,7 +10,8 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATEINCLUDECHECK_H #include "../ClangTidyCheck.h" - +#include +#include namespace clang::tidy::readability { /// \brief Find and remove duplicate #include directives. @@ -19,11 +20,15 @@ namespace clang::tidy::readability { /// directives between them are analyzed. class DuplicateIncludeCheck : public ClangTidyCheck { public: - DuplicateIncludeCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + DuplicateIncludeCheck(StringRef Name, ClangTidyContext *Context); void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + std::vector AllowedDuplicateIncludes; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 666865cfb2fcd..01e962e81d52d 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -487,6 +487,10 @@ Changes in existing checks ` check to recognize literal suffixes added in C++23 and C23. +- Improved :doc:`readability-duplicate-include + ` check by introducing the + ``AllowedDuplicateIncludes`` option, which exempts specified headers from + duplicate include warnings. - Improved :doc:`readability-use-concise-preprocessor-directives ` check to generate correct fix-its for forms without a space after the directive. diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst index 4364610787058..a2ad4b53ae69e 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst @@ -28,7 +28,7 @@ Options ------- .. option:: IgnoreHeaders - + A semicolon-separated list of regexes to disable insertion/removal of header files that match this regex as a suffix. E.g., `foo/.*` disables insertion/removal for all headers under the directory `foo`. Default is an diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst index 45df7e1b84f3f..0b2e9a56732bb 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst @@ -10,7 +10,7 @@ then the list of included files is cleared. Examples: .. code-block:: c++ - + #include #include #include @@ -33,3 +33,13 @@ Because of the intervening macro definitions, this code remains unchanged: #define NDEBUG #include "assertion.h" // ...code with assertions disabled + +Options +------- + +.. option:: IgnoreHeaders + + A semicolon-separated list of regexes to allow duplicate inclusion of header + files that match this regex. E.g., `foo/.*` disables + insertion/removal for all headers under the directory `foo`. Default is an + empty string, no headers will be ignored. diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/else-after-return.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/else-after-return.rst index 25fb40856f40c..d1231d71c3135 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/else-after-return.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/else-after-return.rst @@ -63,7 +63,6 @@ Options ------- .. option:: WarnOnUnfixable - When `true`, emit a warning for cases where the check can't output a Fix-It. These can occur with declarations inside the ``else`` branch that would have an extended lifetime if the ``else`` branch was removed. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/other.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/other.h new file mode 100644 index 0000000000000..24f06a40bef15 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/other.h @@ -0,0 +1,2 @@ +// dummy other.h (should warn on duplicate includes) +#pragma once diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_begin.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_begin.h new file mode 100644 index 0000000000000..373b75e537fbc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_begin.h @@ -0,0 +1,2 @@ +// dummy pack_begin.h (allowed duplicate) +#pragma once diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_end.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_end.h new file mode 100644 index 0000000000000..b8d0fd2073125 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_end.h @@ -0,0 +1,2 @@ +// dummy pack_end.h (allowed duplicate) +#pragma once diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-allowed.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-allowed.cpp new file mode 100644 index 0000000000000..e437d27eea006 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-allowed.cpp @@ -0,0 +1,19 @@ +// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \ +// RUN: -header-filter='' \ +// RUN: -config='{CheckOptions: [{key: readability-duplicate-include.IgnoreHeaders, value: "pack_begin.h;pack_end.h"}]}' \ +// RUN: -- -I %S/Inputs/duplicate-include-allowed +// +// This test lives in test/clang-tidy/checkers/ +// Inputs for allowed duplicate includes are in test/clang-tidy/checkers/Inputs/duplicate-include-allowed + +#include "pack_begin.h" +#include "pack_begin.h" +// No warning expected + +#include "other.h" +#include "other.h" +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicate include [readability-duplicate-include] + +#include "pack_end.h" +#include "pack_end.h" +// No warning expected