Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
68 changes: 62 additions & 6 deletions clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <memory>
#include <vector>

namespace clang::tidy::readability {

Expand All @@ -33,11 +38,8 @@ 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,
Expand All @@ -62,10 +64,25 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
SmallVector<FileList> Files;
DuplicateIncludeCheck &Check;
const SourceManager &SM;
std::vector<llvm::Regex> AllowedDuplicateRegex;

bool IsAllowedDuplicateInclude(StringRef TokenName);
};

} // namespace

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,
Expand All @@ -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.
Expand All @@ -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 &reg : 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<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, "IgnoreHeaders",
llvm::join(AllowedDuplicateIncludes, ";"));
}
} // namespace clang::tidy::readability
11 changes: 8 additions & 3 deletions clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATEINCLUDECHECK_H

#include "../ClangTidyCheck.h"

#include <string>
#include <vector>
namespace clang::tidy::readability {

/// \brief Find and remove duplicate #include directives.
Expand All @@ -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<std::string> AllowedDuplicateIncludes;
};

} // namespace clang::tidy::readability
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,10 @@ 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.
- 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ then the list of included files is cleared.
Examples:

.. code-block:: c++

#include <memory>
#include <vector>
#include <memory>
Expand All @@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// dummy other.h (should warn on duplicate includes)
#pragma once
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// dummy pack_begin.h (allowed duplicate)
#pragma once
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// dummy pack_end.h (allowed duplicate)
#pragma once
Original file line number Diff line number Diff line change
@@ -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