diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp index cc9ae471a926d..4842fe22a8890 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp @@ -7,10 +7,12 @@ //===----------------------------------------------------------------------===// #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/Support/Regex.h" #include namespace clang::tidy::readability { @@ -33,11 +35,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, + llvm::ArrayRef IgnoredList); void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -62,10 +61,31 @@ class DuplicateIncludeCallbacks : public PPCallbacks { SmallVector Files; DuplicateIncludeCheck &Check; const SourceManager &SM; + SmallVector AllowedRegexes; }; } // namespace +DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoredFilesList(utils::options::parseStringList( + Options.get("IgnoredFilesList", ""))) {} + +DuplicateIncludeCallbacks::DuplicateIncludeCallbacks( + DuplicateIncludeCheck &Check, const SourceManager &SM, + llvm::ArrayRef IgnoredList) + : Check(Check), SM(SM) { + // The main file doesn't participate in the FileChanged notification. + Files.emplace_back(); + + AllowedRegexes.reserve(IgnoredList.size()); + for (const StringRef &It : IgnoredList) { + if (!It.empty()) + AllowedRegexes.emplace_back(It); + } +} + void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -78,7 +98,7 @@ void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc, void DuplicateIncludeCallbacks::InclusionDirective( SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, - bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File, + bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef /*File*/, StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule, bool ModuleImported, SrcMgr::CharacteristicKind FileType) { // Skip includes behind macros @@ -86,6 +106,10 @@ void DuplicateIncludeCallbacks::InclusionDirective( FilenameRange.getEnd().isMacroID()) return; if (llvm::is_contained(Files.back(), FileName)) { + if (llvm::any_of(AllowedRegexes, [&FileName](const llvm::Regex &R) { + return R.match(FileName); + })) + return; // We want to delete the entire line, so make sure that [Start,End] covers // everything. const SourceLocation Start = @@ -111,7 +135,13 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok, void DuplicateIncludeCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique(*this, SM)); + PP->addPPCallbacks( + std::make_unique(*this, SM, IgnoredFilesList)); +} + +void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IgnoredFilesList", + utils::options::serializeStringList(IgnoredFilesList)); } } // 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..1ee5b1c209ff7 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h @@ -19,11 +19,16 @@ 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: + // Semicolon-separated list of regexes or file names to ignore from duplicate + // warnings. + const std::vector IgnoredFilesList; }; } // namespace clang::tidy::readability diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4096624923858..52ffc0f82537b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -316,7 +316,7 @@ Changes in existing checks exceptions from captures are now diagnosed, exceptions in the bodies of lambdas that aren't actually invoked are not. Additionally, fixed an issue where the check wouldn't diagnose throws in arguments to functions or - constructors. Added fine-grained configuration via options + constructors. Added fine-grained configuration via options `CheckDestructors`, `CheckMoveMemberFunctions`, `CheckMain`, `CheckedSwapFunctions`, and `CheckNothrowFunctions`. @@ -505,6 +505,11 @@ Changes in existing checks ignoring default constructors with user provided arguments and adding detection in container's method except ``empty``. +- Improved :doc:`readability-duplicate-include + ` check by adding + the ``IgnoredFilesList`` option (semicolon-separated list of regexes or + filenames) to allow intentional duplicates. + - Improved :doc:`readability-identifier-naming ` check by ignoring declarations and macros in system headers. The documentation is also improved 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..28a4991a922f8 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 @@ -33,3 +33,12 @@ Because of the intervening macro definitions, this code remains unchanged: #define NDEBUG #include "assertion.h" // ...code with assertions disabled + +Options +------- + +.. option:: IgnoredFilesList + + A semicolon-separated list of regular expressions or filenames that are + allowed to be included multiple times without diagnostics. Matching is + performed against the textual include name. Default is an empty string. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_begin.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_begin.h new file mode 100644 index 0000000000000..fc9369c0cfb8f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_begin.h @@ -0,0 +1 @@ +// Intentionally unguarded begin-pack header used in tests diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_end.h b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_end.h new file mode 100644 index 0000000000000..78fd0a9adf475 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include/pack_end.h @@ -0,0 +1 @@ +// Intentionally unguarded end-pack header used in tests diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-ignored-files.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-ignored-files.cpp new file mode 100644 index 0000000000000..cdc2f44432d76 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-ignored-files.cpp @@ -0,0 +1,24 @@ +// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \ +// RUN: -config="{CheckOptions: {readability-duplicate-include.IgnoredFilesList: 'pack_.*\\.h'}}" \ +// RUN: -header-filter='' -- -I %S/Inputs/duplicate-include + +int g; +#include "duplicate-include.h" +int h; +#include "duplicate-include.h" +int i; +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include] +// CHECK-FIXES: int g; +// CHECK-FIXES-NEXT: #include "duplicate-include.h" +// CHECK-FIXES-NEXT: int h; +// CHECK-FIXES-NEXT: int i; + +#include "pack_begin.h" +struct A { int x; }; +#include "pack_end.h" + +#include "pack_begin.h" +struct B { int x; }; +#include "pack_end.h" + +// No warning here.