Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <memory>

namespace clang::tidy::readability {
Expand All @@ -33,10 +35,17 @@ using FileList = SmallVector<StringRef>;
class DuplicateIncludeCallbacks : public PPCallbacks {
public:
DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check,
const SourceManager &SM)
const SourceManager &SM,
const std::vector<StringRef> &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 FileChanged(SourceLocation Loc, FileChangeReason Reason,
Expand All @@ -62,10 +71,32 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
SmallVector<FileList> Files;
DuplicateIncludeCheck &Check;
const SourceManager &SM;
std::vector<llvm::Regex> AllowedRegexes;

bool isAllowedDuplicate(StringRef FileName, OptionalFileEntryRef File) const {
if (llvm::any_of(AllowedRegexes,
[&](const llvm::Regex &R) { return R.match(FileName); }))
return true;

if (File) {
const StringRef Resolved = File->getName();
if (llvm::any_of(AllowedRegexes,
[&](const llvm::Regex &R) { return R.match(Resolved); }))
return true;
}

return false;
}
};

} // namespace

DuplicateIncludeCheck::DuplicateIncludeCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
IgnoredFilesList(utils::options::parseStringList(
Options.get("IgnoredFilesList", ""))) {}

void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
Expand All @@ -86,6 +117,9 @@ void DuplicateIncludeCallbacks::InclusionDirective(
FilenameRange.getEnd().isMacroID())
return;
if (llvm::is_contained(Files.back(), FileName)) {
if (isAllowedDuplicate(FileName, File)) {
return;
}
// We want to delete the entire line, so make sure that [Start,End] covers
// everything.
const SourceLocation Start =
Expand All @@ -111,7 +145,13 @@ void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok,

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, IgnoredFilesList));
}

void DuplicateIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoredFilesList",
utils::options::serializeStringList(IgnoredFilesList));
}

} // namespace clang::tidy::readability
10 changes: 8 additions & 2 deletions clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATEINCLUDECHECK_H

#include "../ClangTidyCheck.h"
#include <vector>

namespace clang::tidy::readability {

Expand All @@ -19,11 +20,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<StringRef> IgnoredFilesList;
};

} // namespace clang::tidy::readability
Expand Down
7 changes: 6 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down Expand Up @@ -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
<clang-tidy/checks/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
<clang-tidy/checks/readability/identifier-naming>` check by ignoring
declarations and macros in system headers. The documentation is also improved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ Looks for duplicate includes and removes them. The check maintains a list of
included files and looks for duplicates. If a macro is defined or undefined
then the list of included files is cleared.

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. If the header can be
resolved, its full path is also matched against the patterns.
Default is empty.

Examples:

.. code-block:: c++
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Intentionally unguarded begin-pack header used in tests
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Intentionally unguarded end-pack header used in tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %check_clang_tidy %s readability-duplicate-include %t -- \
// RUN: -config="{CheckOptions: {readability-duplicate-include.IgnoredFilesList: 'pack_.*\\.h'}}" \
// RUN: -- -I %S/Inputs/duplicate-include

#include "pack_begin.h"
struct A { int x; };
#include "pack_end.h"

// no warning
Loading