https://github.com/lakshsidhu04 updated https://github.com/llvm/llvm-project/pull/167046
>From 669a5aed36326b3439fadf55a4b0c3f5f7219dc4 Mon Sep 17 00:00:00 2001 From: lakshsidhu04 <[email protected]> Date: Sat, 8 Nov 2025 04:35:26 +0530 Subject: [PATCH 1/3] clang tidy: add header ignore option to include duplicate files --- .../misc/HeaderIncludeCycleCheck.cpp | 2 +- .../readability/DuplicateIncludeCheck.cpp | 91 ++++++++++++++++--- .../readability/DuplicateIncludeCheck.h | 10 +- 3 files changed, 88 insertions(+), 15 deletions(-) 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 0237c057afed5..5ed783bcef824 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp @@ -11,7 +11,12 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/Support/Regex.h" +#include "llvm/Support/Path.h" +#include "../utils/OptionsUtils.h" +#include "llvm/ADT/StringExtras.h" #include <memory> +#include <vector> namespace clang::tidy::readability { @@ -33,12 +38,9 @@ using FileList = SmallVector<StringRef>; 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<std::string> &AllowedStrings); + void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, FileID PrevFID) override; @@ -62,14 +64,28 @@ class DuplicateIncludeCallbacks : public PPCallbacks { SmallVector<FileList> Files; DuplicateIncludeCheck &Check; const SourceManager &SM; + std::vector<llvm::Regex> AllowedDuplicateRegex; + + bool IsAllowedDuplicateInclude(StringRef TokenName, OptionalFileEntryRef File, + StringRef RelativePath); }; } // namespace -void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc, - FileChangeReason Reason, - SrcMgr::CharacteristicKind FileType, - FileID PrevFID) { +DuplicateIncludeCallbacks::DuplicateIncludeCallbacks( + DuplicateIncludeCheck &Check, const SourceManager &SM, + const std::vector<std::string> &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, FileID PrevFID) { if (Reason == EnterFile) Files.emplace_back(); else if (Reason == ExitFile) @@ -85,6 +101,14 @@ void DuplicateIncludeCallbacks::InclusionDirective( if (FilenameRange.getBegin().isMacroID() || FilenameRange.getEnd().isMacroID()) return; + + // if duplicate allowed, record and return + if(IsAllowedDuplicateInclude(FileName, File, RelativePath)) + { + 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,54 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok, Files.back().clear(); } +bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude(StringRef TokenName, + OptionalFileEntryRef File, + StringRef RelativePath) { + SmallVector<StringRef, 3> matchArguments; + matchArguments.push_back(TokenName); + + if (!RelativePath.empty()) + matchArguments.push_back(llvm::sys::path::filename(RelativePath)); + + if (File) { + StringRef RealPath = File->getFileEntry().tryGetRealPathName(); + if (!RealPath.empty()) + matchArguments.push_back(llvm::sys::path::filename(RealPath)); + } + + // try to match with each regex + for (const llvm::Regex ® : AllowedDuplicateRegex) { + for (StringRef arg : matchArguments) { + if (reg.match(arg)) + return true; + } + } + return false; +} + +DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) { + std::string Raw = Options.get("AllowedDuplicateIncludes", "").str(); + if (!Raw.empty()) { + SmallVector<StringRef, 4> 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<DuplicateIncludeCallbacks>(*this, SM)); + PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(*this, SM, AllowedDuplicateIncludes)); } +void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowedDuplicateIncludes", + 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 297999cf4f921..688c2ad4d30f1 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_DUPLICATE_INCLUDE_CHECK_H #include "../ClangTidyCheck.h" - +#include <vector> +#include <string> namespace clang::tidy::readability { /// \brief Find and remove duplicate #include directives. @@ -19,11 +20,14 @@ 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<std::string> AllowedDuplicateIncludes; }; } // namespace clang::tidy::readability >From b29140ab5dc9afab9fda67df1888e135e19d9e1a Mon Sep 17 00:00:00 2001 From: lakshsidhu04 <[email protected]> Date: Sat, 8 Nov 2025 21:49:28 +0530 Subject: [PATCH 2/3] fix: formatted the check code, added tests and documentation --- .../readability/DuplicateIncludeCheck.cpp | 47 ++++++++++--------- .../readability/DuplicateIncludeCheck.h | 5 +- clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../checks/readability/duplicate-include.rst | 20 +++++++- .../Inputs/duplicate-include-allowed/other.h | 2 + .../duplicate-include-allowed/pack_begin.h | 2 + .../duplicate-include-allowed/pack_end.h | 2 + .../readability/duplicate-include-allowed.cpp | 19 ++++++++ 8 files changed, 76 insertions(+), 26 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/other.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_begin.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/duplicate-include-allowed/pack_end.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/duplicate-include-allowed.cpp diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp index 5ed783bcef824..59b1b13267ccb 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp @@ -7,14 +7,14 @@ //===----------------------------------------------------------------------===// #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 "llvm/Support/Path.h" -#include "../utils/OptionsUtils.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/Regex.h" #include <memory> #include <vector> @@ -38,9 +38,9 @@ using FileList = SmallVector<StringRef>; class DuplicateIncludeCallbacks : public PPCallbacks { public: DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check, - const SourceManager &SM, + const SourceManager &SM, const std::vector<std::string> &AllowedStrings); - + void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, FileID PrevFID) override; @@ -74,7 +74,8 @@ class DuplicateIncludeCallbacks : public PPCallbacks { DuplicateIncludeCallbacks::DuplicateIncludeCallbacks( DuplicateIncludeCheck &Check, const SourceManager &SM, - const std::vector<std::string> &AllowedStrings) : Check(Check), SM(SM) { + const std::vector<std::string> &AllowedStrings) + : Check(Check), SM(SM) { // The main file doesn't participate in the FileChanged notification. Files.emplace_back(); AllowedDuplicateRegex.reserve(AllowedStrings.size()); @@ -83,9 +84,10 @@ DuplicateIncludeCallbacks::DuplicateIncludeCallbacks( } } -void DuplicateIncludeCallbacks::FileChanged( - SourceLocation Loc, FileChangeReason Reason, - SrcMgr::CharacteristicKind FileType, FileID PrevFID) { +void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc, + FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) { if (Reason == EnterFile) Files.emplace_back(); else if (Reason == ExitFile) @@ -101,12 +103,11 @@ void DuplicateIncludeCallbacks::InclusionDirective( if (FilenameRange.getBegin().isMacroID() || FilenameRange.getEnd().isMacroID()) return; - + // if duplicate allowed, record and return - if(IsAllowedDuplicateInclude(FileName, File, RelativePath)) - { - Files.back().push_back(FileName); - return; + if (IsAllowedDuplicateInclude(FileName, File, RelativePath)) { + Files.back().push_back(FileName); + return; } if (llvm::is_contained(Files.back(), FileName)) { @@ -133,21 +134,20 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok, Files.back().clear(); } -bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude(StringRef TokenName, - OptionalFileEntryRef File, - StringRef RelativePath) { +bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude( + StringRef TokenName, OptionalFileEntryRef File, StringRef RelativePath) { SmallVector<StringRef, 3> matchArguments; matchArguments.push_back(TokenName); - + if (!RelativePath.empty()) matchArguments.push_back(llvm::sys::path::filename(RelativePath)); - + if (File) { StringRef RealPath = File->getFileEntry().tryGetRealPathName(); if (!RealPath.empty()) matchArguments.push_back(llvm::sys::path::filename(RealPath)); } - + // try to match with each regex for (const llvm::Regex ® : AllowedDuplicateRegex) { for (StringRef arg : matchArguments) { @@ -165,9 +165,9 @@ DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name, if (!Raw.empty()) { SmallVector<StringRef, 4> StringParts; StringRef(Raw).split(StringParts, ',', -1, false); - + for (StringRef Part : StringParts) { - Part = Part.trim(); + Part = Part.trim(); if (!Part.empty()) AllowedDuplicateIncludes.push_back(Part.str()); } @@ -176,7 +176,8 @@ DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name, void DuplicateIncludeCheck::registerPPCallbacks( const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(*this, SM, AllowedDuplicateIncludes)); + PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>( + *this, SM, AllowedDuplicateIncludes)); } void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h index 688c2ad4d30f1..fc258bc1c30c5 100644 --- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h +++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h @@ -10,8 +10,8 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H #include "../ClangTidyCheck.h" -#include <vector> #include <string> +#include <vector> namespace clang::tidy::readability { /// \brief Find and remove duplicate #include directives. @@ -24,8 +24,9 @@ class DuplicateIncludeCheck : public ClangTidyCheck { void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; - + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + private: std::vector<std::string> AllowedDuplicateIncludes; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8ba1d1a78bf91..2a612a234ffd8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -480,6 +480,11 @@ Changes in existing checks <clang-tidy/checks/readability/uppercase-literal-suffix>` check to recognize literal suffixes added in C++23 and C23. +- Improved :doc:`readability-duplicate-include + <clang-tidy/checks/readability/duplicate-include>` check by introducing the + ``AllowedDuplicateIncludes`` option, which exempts specified headers from + duplicate include warnings. + Removed checks ^^^^^^^^^^^^^^ 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..b86c53ac26dfd 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 <memory> #include <vector> #include <memory> @@ -33,3 +33,21 @@ Because of the intervening macro definitions, this code remains unchanged: #define NDEBUG #include "assertion.h" // ...code with assertions disabled + +Option: ``AllowedDuplicateIncludes`` +------------------------------------ + +Headers listed in this option are exempt from warnings. For example: + +.. code-block:: c++ + + -config='{CheckOptions: [{key: readability-duplicate-include.AllowedDuplicateIncludes, value: "pack_begin.h,pack_end.h"}]}' + +This allows regex matches with ``pack_begin.h`` and ``pack_end.h`` to be included multiple times +without triggering diagnostics. + +Notes +----- + +- Only direct includes in the current translation unit are checked. +- Useful for removing redundant includes and improving compile times in large codebases. 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..c7a26e95ced51 --- /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 \ No newline at end of file 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..def9e2ddbca80 --- /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.AllowedDuplicateIncludes, 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 >From 0658b029c1bd9114d3a33cd30e2ff03718733c0c Mon Sep 17 00:00:00 2001 From: Lakshdeep Singh <[email protected]> Date: Sun, 9 Nov 2025 02:39:09 +0530 Subject: [PATCH 3/3] Update clang-tools-extra/docs/ReleaseNotes.rst Co-authored-by: Baranov Victor <[email protected]> --- clang-tools-extra/docs/ReleaseNotes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 01e962e81d52d..7edf045596a81 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -491,6 +491,7 @@ Changes in existing checks <clang-tidy/checks/readability/duplicate-include>` check by introducing the ``AllowedDuplicateIncludes`` option, which exempts specified headers from duplicate include warnings. + - Improved :doc:`readability-use-concise-preprocessor-directives <clang-tidy/checks/readability/use-concise-preprocessor-directives>` check to generate correct fix-its for forms without a space after the directive. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
