Skip to content

Commit

Permalink
[clang-tidy][NFC] Remove duplicated code
Browse files Browse the repository at this point in the history
Remove duplicated matchers by moving some of them to
utils/Matchers.h. Add some anonymous namespaces and
renamed some code to avoid ODR issues.
  • Loading branch information
PiotrZSL committed Mar 31, 2024
1 parent d12e45a commit b6f6be4
Show file tree
Hide file tree
Showing 45 changed files with 298 additions and 288 deletions.
5 changes: 5 additions & 0 deletions clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_ABSEILMATCHER_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_ABSEILMATCHER_H

#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include <algorithm>
Expand Down Expand Up @@ -57,3 +60,5 @@ AST_POLYMORPHIC_MATCHER(
}

} // namespace clang::ast_matchers

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_ABSEILMATCHER_H
14 changes: 4 additions & 10 deletions clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "DurationFactoryFloatCheck.h"
#include "../utils/LexerUtils.h"
#include "DurationRewriter.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
Expand All @@ -18,15 +19,6 @@ using namespace clang::ast_matchers;

namespace clang::tidy::abseil {

// Returns `true` if `Range` is inside a macro definition.
static bool insideMacroDefinition(const MatchFinder::MatchResult &Result,
SourceRange Range) {
return !clang::Lexer::makeFileCharRange(
clang::CharSourceRange::getCharRange(Range),
*Result.SourceManager, Result.Context->getLangOpts())
.isValid();
}

void DurationFactoryFloatCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(callee(functionDecl(DurationFactoryFunction())),
Expand All @@ -45,7 +37,9 @@ void DurationFactoryFloatCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("call");

// Don't try and replace things inside of macro definitions.
if (insideMacroDefinition(Result, MatchedCall->getSourceRange()))
if (tidy::utils::lexer::insideMacroDefinition(MatchedCall->getSourceRange(),
*Result.SourceManager,
Result.Context->getLangOpts()))
return;

const Expr *Arg = MatchedCall->getArg(0)->IgnoreImpCasts();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@ using ::clang::transformer::makeRule;
using ::clang::transformer::node;
using ::clang::transformer::RewriteRuleWith;

namespace {

AST_MATCHER(Type, isCharType) { return Node.isCharType(); }
} // namespace

static const char DefaultStringLikeClasses[] = "::std::basic_string;"
static const char StringLikeClassesDefault[] = "::std::basic_string;"
"::std::basic_string_view;"
"::absl::string_view";
static const char DefaultAbseilStringsMatchHeader[] = "absl/strings/match.h";
static const char AbseilStringsMatchHeaderDefault[] = "absl/strings/match.h";

static transformer::RewriteRuleWith<std::string>
makeRewriteRule(ArrayRef<StringRef> StringLikeClassNames,
Expand Down Expand Up @@ -84,9 +87,9 @@ StringFindStrContainsCheck::StringFindStrContainsCheck(
StringRef Name, ClangTidyContext *Context)
: TransformerClangTidyCheck(Name, Context),
StringLikeClassesOption(utils::options::parseStringList(
Options.get("StringLikeClasses", DefaultStringLikeClasses))),
Options.get("StringLikeClasses", StringLikeClassesDefault))),
AbseilStringsMatchHeaderOption(Options.get(
"AbseilStringsMatchHeader", DefaultAbseilStringsMatchHeader)) {
"AbseilStringsMatchHeader", AbseilStringsMatchHeaderDefault)) {
setRule(
makeRewriteRule(StringLikeClassesOption, AbseilStringsMatchHeaderOption));
}
Expand Down
22 changes: 10 additions & 12 deletions clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "TimeSubtractionCheck.h"
#include "../utils/LexerUtils.h"
#include "DurationRewriter.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
Expand All @@ -18,15 +19,6 @@ using namespace clang::ast_matchers;

namespace clang::tidy::abseil {

// Returns `true` if `Range` is inside a macro definition.
static bool insideMacroDefinition(const MatchFinder::MatchResult &Result,
SourceRange Range) {
return !clang::Lexer::makeFileCharRange(
clang::CharSourceRange::getCharRange(Range),
*Result.SourceManager, Result.Context->getLangOpts())
.isValid();
}

static bool isConstructorAssignment(const MatchFinder::MatchResult &Result,
const Expr *Node) {
// For C++14 and earlier there are elidable constructors that must be matched
Expand Down Expand Up @@ -130,7 +122,9 @@ void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) {
const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binop");
std::string InverseName =
Result.Nodes.getNodeAs<FunctionDecl>("func_decl")->getNameAsString();
if (insideMacroDefinition(Result, BinOp->getSourceRange()))
if (tidy::utils::lexer::insideMacroDefinition(BinOp->getSourceRange(),
*Result.SourceManager,
Result.Context->getLangOpts()))
return;

std::optional<DurationScale> Scale = getScaleForTimeInverse(InverseName);
Expand All @@ -139,7 +133,9 @@ void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) {

const auto *OuterCall = Result.Nodes.getNodeAs<CallExpr>("outer_call");
if (OuterCall) {
if (insideMacroDefinition(Result, OuterCall->getSourceRange()))
if (tidy::utils::lexer::insideMacroDefinition(
OuterCall->getSourceRange(), *Result.SourceManager,
Result.Context->getLangOpts()))
return;

// We're working with the first case of matcher, and need to replace the
Expand All @@ -165,7 +161,9 @@ void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) {
.bind("arg"))),
*BinOp, *Result.Context));
if (MaybeCallArg && MaybeCallArg->getArg(0)->IgnoreImpCasts() == BinOp &&
!insideMacroDefinition(Result, MaybeCallArg->getSourceRange())) {
!tidy::utils::lexer::insideMacroDefinition(
MaybeCallArg->getSourceRange(), *Result.SourceManager,
Result.Context->getLangOpts())) {
// Handle the case where the matched expression is inside a call which
// converts it from the inverse to a Duration. In this case, we replace
// the outer with just the subtraction expression, which gives the right
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ void UpgradeDurationConversionsCheck::check(

// We gather source locations from template matches not in template
// instantiations for future matches.
internal::Matcher<Stmt> IsInsideTemplate =
hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl())));
auto IsInsideTemplate = stmt(
hasAncestor(decl(anyOf(classTemplateDecl(), functionTemplateDecl()))));
if (!match(IsInsideTemplate, *ArgExpr, *Result.Context).empty())
MatchedTemplateLocations.insert(Loc);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ void BadSignalToKillThreadCheck::registerMatchers(MatchFinder *Finder) {
this);
}

static Preprocessor *PP;

void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
const auto IsSigterm = [](const auto &KeyValue) -> bool {
return KeyValue.first->getName() == "SIGTERM" &&
KeyValue.first->hasMacroDefinition();
};
const auto TryExpandAsInteger =
[](Preprocessor::macro_iterator It) -> std::optional<unsigned> {
[this](Preprocessor::macro_iterator It) -> std::optional<unsigned> {
if (It == PP->macro_end())
return std::nullopt;
const MacroInfo *MI = PP->getMacroInfo(It->first);
Expand Down Expand Up @@ -64,8 +62,8 @@ void BadSignalToKillThreadCheck::check(const MatchFinder::MatchResult &Result) {
}

void BadSignalToKillThreadCheck::registerPPCallbacks(
const SourceManager &SM, Preprocessor *Pp, Preprocessor *ModuleExpanderPP) {
PP = Pp;
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
this->PP = PP;
}

} // namespace clang::tidy::bugprone
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ class BadSignalToKillThreadCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;

private:
std::optional<unsigned> SigtermValue;
Preprocessor *PP = nullptr;
};

} // namespace clang::tidy::bugprone
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ using namespace clang::ast_matchers;

namespace clang::tidy::bugprone {

namespace {

AST_MATCHER(clang::VarDecl, hasConstantDeclaration) {
const Expr *Init = Node.getInit();
if (Init && !Init->isValueDependent()) {
Expand All @@ -25,6 +27,8 @@ AST_MATCHER(clang::VarDecl, hasConstantDeclaration) {
return false;
}

} // namespace

DynamicStaticInitializersCheck::DynamicStaticInitializersCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,8 @@ static bool prefixSuffixCoverUnderThreshold(std::size_t Threshold,

} // namespace filter

namespace {

/// Matches functions that have at least the specified amount of parameters.
AST_MATCHER_P(FunctionDecl, parameterCountGE, unsigned, N) {
return Node.getNumParams() >= N;
Expand All @@ -1903,6 +1905,8 @@ AST_MATCHER(FunctionDecl, isOverloadedUnaryOrBinaryOperator) {
}
}

} // namespace

/// Returns the DefaultMinimumLength if the Value of requested minimum length
/// is less than 2. Minimum lengths of 0 or 1 are not accepted.
static inline unsigned clampMinimumLength(const unsigned Value) {
Expand Down
5 changes: 3 additions & 2 deletions clang-tools-extra/clang-tidy/bugprone/EmptyCatchCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ using ::clang::ast_matchers::internal::Matcher;
namespace clang::tidy::bugprone {

namespace {
AST_MATCHER(CXXCatchStmt, isInMacro) {
AST_MATCHER(CXXCatchStmt, isCatchInMacro) {
return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID() ||
Node.getCatchLoc().isMacroID();
}
Expand Down Expand Up @@ -89,7 +89,8 @@ void EmptyCatchCheck::registerMatchers(MatchFinder *Finder) {
hasCanonicalType(AllowedNamedExceptionTypes)));

Finder->addMatcher(
cxxCatchStmt(unless(isExpansionInSystemHeader()), unless(isInMacro()),
cxxCatchStmt(unless(isExpansionInSystemHeader()),
unless(isCatchInMacro()),
unless(hasCaughtType(IgnoredExceptionType)),
hasHandler(compoundStmt(
statementCountIs(0),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ using namespace clang::ast_matchers;

namespace clang::tidy::bugprone {

namespace {
AST_MATCHER(BinaryOperator, isLogicalOperator) { return Node.isLogicalOp(); }

AST_MATCHER(UnaryOperator, isUnaryPrePostOperator) {
Expand All @@ -26,6 +27,8 @@ AST_MATCHER(CXXOperatorCallExpr, isPrePostOperator) {
Node.getOperator() == OO_MinusMinus;
}

} // namespace

void IncDecInConditionsCheck::registerMatchers(MatchFinder *Finder) {
auto OperatorMatcher = expr(
anyOf(binaryOperator(anyOf(isComparisonOperator(), isLogicalOperator())),
Expand Down
8 changes: 4 additions & 4 deletions clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ AST_MATCHER(FunctionType, typeHasNoReturnAttr) {
} // namespace ast_matchers
namespace tidy::bugprone {

static internal::Matcher<Stmt>
loopEndingStmt(internal::Matcher<Stmt> Internal) {
internal::Matcher<QualType> isNoReturnFunType =
static ast_matchers::internal::Matcher<Stmt>
loopEndingStmt(ast_matchers::internal::Matcher<Stmt> Internal) {
ast_matchers::internal::Matcher<QualType> isNoReturnFunType =
ignoringParens(functionType(typeHasNoReturnAttr()));
internal::Matcher<Decl> isNoReturnDecl =
ast_matchers::internal::Matcher<Decl> isNoReturnDecl =
anyOf(declHasNoReturnAttr(), functionDecl(hasType(isNoReturnFunType)),
varDecl(hasType(blockPointerType(pointee(isNoReturnFunType)))));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "MultipleNewInOneExpressionCheck.h"
#include "../utils/Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
Expand Down Expand Up @@ -51,29 +52,6 @@ bool isExprValueStored(const Expr *E, ASTContext &C) {

} // namespace

AST_MATCHER_P(CXXTryStmt, hasHandlerFor,
ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
for (unsigned NH = Node.getNumHandlers(), I = 0; I < NH; ++I) {
const CXXCatchStmt *CatchS = Node.getHandler(I);
// Check for generic catch handler (match anything).
if (CatchS->getCaughtType().isNull())
return true;
ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
if (InnerMatcher.matches(CatchS->getCaughtType(), Finder, &Result)) {
*Builder = std::move(Result);
return true;
}
}
return false;
}

AST_MATCHER(CXXNewExpr, mayThrow) {
FunctionDecl *OperatorNew = Node.getOperatorNew();
if (!OperatorNew)
return false;
return !OperatorNew->getType()->castAs<FunctionProtoType>()->isNothrow();
}

void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) {
auto BadAllocType =
recordType(hasDeclaration(cxxRecordDecl(hasName("::std::bad_alloc"))));
Expand All @@ -85,9 +63,10 @@ void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) {
auto CatchBadAllocType =
qualType(hasCanonicalType(anyOf(BadAllocType, BadAllocReferenceType,
ExceptionType, ExceptionReferenceType)));
auto BadAllocCatchingTryBlock = cxxTryStmt(hasHandlerFor(CatchBadAllocType));
auto BadAllocCatchingTryBlock =
cxxTryStmt(matchers::hasHandlerFor(CatchBadAllocType));

auto NewExprMayThrow = cxxNewExpr(mayThrow());
auto NewExprMayThrow = cxxNewExpr(matchers::mayThrow());
auto HasNewExpr1 = expr(anyOf(NewExprMayThrow.bind("new1"),
hasDescendant(NewExprMayThrow.bind("new1"))));
auto HasNewExpr2 = expr(anyOf(NewExprMayThrow.bind("new2"),
Expand Down Expand Up @@ -115,7 +94,7 @@ void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) {
hasAncestor(BadAllocCatchingTryBlock)),
this);
Finder->addMatcher(
cxxNewExpr(mayThrow(),
cxxNewExpr(matchers::mayThrow(),
hasDescendant(NewExprMayThrow.bind("new2_in_new1")),
hasAncestor(BadAllocCatchingTryBlock))
.bind("new1"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@ namespace clang::tidy::bugprone {

namespace {

AST_MATCHER(Expr, isInMacro) { return Node.getBeginLoc().isMacroID(); }
AST_MATCHER(Expr, isExprInMacro) { return Node.getBeginLoc().isMacroID(); }

} // namespace

/// Find the next statement after `S`.
const Stmt *nextStmt(const MatchFinder::MatchResult &Result, const Stmt *S) {
static const Stmt *nextStmt(const MatchFinder::MatchResult &Result,
const Stmt *S) {
auto Parents = Result.Context->getParents(*S);
if (Parents.empty())
return nullptr;
Expand All @@ -40,8 +43,8 @@ using ExpansionRanges = std::vector<SourceRange>;
/// \brief Get all the macro expansion ranges related to `Loc`.
///
/// The result is ordered from most inner to most outer.
ExpansionRanges getExpansionRanges(SourceLocation Loc,
const MatchFinder::MatchResult &Result) {
static ExpansionRanges
getExpansionRanges(SourceLocation Loc, const MatchFinder::MatchResult &Result) {
ExpansionRanges Locs;
while (Loc.isMacroID()) {
Locs.push_back(
Expand All @@ -51,10 +54,9 @@ ExpansionRanges getExpansionRanges(SourceLocation Loc,
return Locs;
}

} // namespace

void MultipleStatementMacroCheck::registerMatchers(MatchFinder *Finder) {
const auto Inner = expr(isInMacro(), unless(compoundStmt())).bind("inner");
const auto Inner =
expr(isExprInMacro(), unless(compoundStmt())).bind("inner");
Finder->addMatcher(
stmt(anyOf(ifStmt(hasThen(Inner)), ifStmt(hasElse(Inner)).bind("else"),
whileStmt(hasBody(Inner)), forStmt(hasBody(Inner))))
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,12 @@ SourceRange getSourceRangeOfStmt(const Stmt *S, ASTContext &Ctx) {
return P.getSourceRange();
}

} // namespace

AST_MATCHER(FunctionDecl, isStandardFunction) {
return isStandardFunction(&Node);
}

} // namespace

SignalHandlerCheck::SignalHandlerCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
Expand Down

0 comments on commit b6f6be4

Please sign in to comment.