Skip to content

Commit b29140a

Browse files
committed
fix: formatted the check code, added tests and documentation
1 parent 669a5ae commit b29140a

File tree

8 files changed

+76
-26
lines changed

8 files changed

+76
-26
lines changed

clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "DuplicateIncludeCheck.h"
10+
#include "../utils/OptionsUtils.h"
1011
#include "clang/Frontend/CompilerInstance.h"
1112
#include "clang/Lex/Preprocessor.h"
1213
#include "llvm/ADT/STLExtras.h"
1314
#include "llvm/ADT/SmallVector.h"
14-
#include "llvm/Support/Regex.h"
15-
#include "llvm/Support/Path.h"
16-
#include "../utils/OptionsUtils.h"
1715
#include "llvm/ADT/StringExtras.h"
16+
#include "llvm/Support/Path.h"
17+
#include "llvm/Support/Regex.h"
1818
#include <memory>
1919
#include <vector>
2020

@@ -38,9 +38,9 @@ using FileList = SmallVector<StringRef>;
3838
class DuplicateIncludeCallbacks : public PPCallbacks {
3939
public:
4040
DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check,
41-
const SourceManager &SM,
41+
const SourceManager &SM,
4242
const std::vector<std::string> &AllowedStrings);
43-
43+
4444
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
4545
SrcMgr::CharacteristicKind FileType,
4646
FileID PrevFID) override;
@@ -74,7 +74,8 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
7474

7575
DuplicateIncludeCallbacks::DuplicateIncludeCallbacks(
7676
DuplicateIncludeCheck &Check, const SourceManager &SM,
77-
const std::vector<std::string> &AllowedStrings) : Check(Check), SM(SM) {
77+
const std::vector<std::string> &AllowedStrings)
78+
: Check(Check), SM(SM) {
7879
// The main file doesn't participate in the FileChanged notification.
7980
Files.emplace_back();
8081
AllowedDuplicateRegex.reserve(AllowedStrings.size());
@@ -83,9 +84,10 @@ DuplicateIncludeCallbacks::DuplicateIncludeCallbacks(
8384
}
8485
}
8586

86-
void DuplicateIncludeCallbacks::FileChanged(
87-
SourceLocation Loc, FileChangeReason Reason,
88-
SrcMgr::CharacteristicKind FileType, FileID PrevFID) {
87+
void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
88+
FileChangeReason Reason,
89+
SrcMgr::CharacteristicKind FileType,
90+
FileID PrevFID) {
8991
if (Reason == EnterFile)
9092
Files.emplace_back();
9193
else if (Reason == ExitFile)
@@ -101,12 +103,11 @@ void DuplicateIncludeCallbacks::InclusionDirective(
101103
if (FilenameRange.getBegin().isMacroID() ||
102104
FilenameRange.getEnd().isMacroID())
103105
return;
104-
106+
105107
// if duplicate allowed, record and return
106-
if(IsAllowedDuplicateInclude(FileName, File, RelativePath))
107-
{
108-
Files.back().push_back(FileName);
109-
return;
108+
if (IsAllowedDuplicateInclude(FileName, File, RelativePath)) {
109+
Files.back().push_back(FileName);
110+
return;
110111
}
111112

112113
if (llvm::is_contained(Files.back(), FileName)) {
@@ -133,21 +134,20 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok,
133134
Files.back().clear();
134135
}
135136

136-
bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude(StringRef TokenName,
137-
OptionalFileEntryRef File,
138-
StringRef RelativePath) {
137+
bool DuplicateIncludeCallbacks::IsAllowedDuplicateInclude(
138+
StringRef TokenName, OptionalFileEntryRef File, StringRef RelativePath) {
139139
SmallVector<StringRef, 3> matchArguments;
140140
matchArguments.push_back(TokenName);
141-
141+
142142
if (!RelativePath.empty())
143143
matchArguments.push_back(llvm::sys::path::filename(RelativePath));
144-
144+
145145
if (File) {
146146
StringRef RealPath = File->getFileEntry().tryGetRealPathName();
147147
if (!RealPath.empty())
148148
matchArguments.push_back(llvm::sys::path::filename(RealPath));
149149
}
150-
150+
151151
// try to match with each regex
152152
for (const llvm::Regex &reg : AllowedDuplicateRegex) {
153153
for (StringRef arg : matchArguments) {
@@ -165,9 +165,9 @@ DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name,
165165
if (!Raw.empty()) {
166166
SmallVector<StringRef, 4> StringParts;
167167
StringRef(Raw).split(StringParts, ',', -1, false);
168-
168+
169169
for (StringRef Part : StringParts) {
170-
Part = Part.trim();
170+
Part = Part.trim();
171171
if (!Part.empty())
172172
AllowedDuplicateIncludes.push_back(Part.str());
173173
}
@@ -176,7 +176,8 @@ DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name,
176176

177177
void DuplicateIncludeCheck::registerPPCallbacks(
178178
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
179-
PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(*this, SM, AllowedDuplicateIncludes));
179+
PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(
180+
*this, SM, AllowedDuplicateIncludes));
180181
}
181182

182183
void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {

clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H
1111

1212
#include "../ClangTidyCheck.h"
13-
#include <vector>
1413
#include <string>
14+
#include <vector>
1515
namespace clang::tidy::readability {
1616

1717
/// \brief Find and remove duplicate #include directives.
@@ -24,8 +24,9 @@ class DuplicateIncludeCheck : public ClangTidyCheck {
2424

2525
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
2626
Preprocessor *ModuleExpanderPP) override;
27-
27+
2828
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
29+
2930
private:
3031
std::vector<std::string> AllowedDuplicateIncludes;
3132
};

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,11 @@ Changes in existing checks
480480
<clang-tidy/checks/readability/uppercase-literal-suffix>` check to recognize
481481
literal suffixes added in C++23 and C23.
482482

483+
- Improved :doc:`readability-duplicate-include
484+
<clang-tidy/checks/readability/duplicate-include>` check by introducing the
485+
``AllowedDuplicateIncludes`` option, which exempts specified headers from
486+
duplicate include warnings.
487+
483488
Removed checks
484489
^^^^^^^^^^^^^^
485490

clang-tools-extra/docs/clang-tidy/checks/readability/duplicate-include.rst

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ then the list of included files is cleared.
1010
Examples:
1111

1212
.. code-block:: c++
13-
13+
1414
#include <memory>
1515
#include <vector>
1616
#include <memory>
@@ -33,3 +33,21 @@ Because of the intervening macro definitions, this code remains unchanged:
3333
#define NDEBUG
3434
#include "assertion.h"
3535
// ...code with assertions disabled
36+
37+
Option: ``AllowedDuplicateIncludes``
38+
------------------------------------
39+
40+
Headers listed in this option are exempt from warnings. For example:
41+
42+
.. code-block:: c++
43+
44+
-config='{CheckOptions: [{key: readability-duplicate-include.AllowedDuplicateIncludes, value: "pack_begin.h,pack_end.h"}]}'
45+
46+
This allows regex matches with ``pack_begin.h`` and ``pack_end.h`` to be included multiple times
47+
without triggering diagnostics.
48+
49+
Notes
50+
-----
51+
52+
- Only direct includes in the current translation unit are checked.
53+
- Useful for removing redundant includes and improving compile times in large codebases.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// dummy other.h (should warn on duplicate includes)
2+
#pragma once
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// dummy pack_begin.h (allowed duplicate)
2+
#pragma once
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// dummy pack_end.h (allowed duplicate)
2+
#pragma once
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \
2+
// RUN: -header-filter='' \
3+
// RUN: -config='{CheckOptions: [{key: readability-duplicate-include.AllowedDuplicateIncludes, value: "pack_begin.h,pack_end.h"}]}' \
4+
// RUN: -- -I %S/Inputs/duplicate-include-allowed
5+
//
6+
// This test lives in test/clang-tidy/checkers/
7+
// Inputs for allowed duplicate includes are in test/clang-tidy/checkers/Inputs/duplicate-include-allowed
8+
9+
#include "pack_begin.h"
10+
#include "pack_begin.h"
11+
// No warning expected
12+
13+
#include "other.h"
14+
#include "other.h"
15+
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicate include [readability-duplicate-include]
16+
17+
#include "pack_end.h"
18+
#include "pack_end.h"
19+
// No warning expected

0 commit comments

Comments
 (0)