Skip to content

Commit edd4800

Browse files
committed
review fixes
1 parent f74066d commit edd4800

File tree

2 files changed

+37
-42
lines changed

2 files changed

+37
-42
lines changed

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.cpp

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,31 @@ namespace clang::tidy::performance {
1616

1717
using utils::decl_ref_expr::allDeclRefExprs;
1818

19+
static const Expr* getLastVarUsage(const VarDecl& Var, const Decl& Func,
20+
ASTContext& Context) {
21+
auto Exprs = allDeclRefExprs(Var, Func, Context);
22+
23+
const Expr* LastExpr = nullptr;
24+
for (const clang::DeclRefExpr* Expr : Exprs) {
25+
if (!LastExpr) LastExpr = Expr;
26+
27+
if (LastExpr->getBeginLoc() < Expr->getBeginLoc()) LastExpr = Expr;
28+
}
29+
30+
return LastExpr;
31+
}
32+
1933
AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
2034
return Node.hasDefinition() && Node.hasTrivialMoveConstructor();
2135
}
2236

23-
void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
37+
void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
2438
auto returnParent =
2539
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
2640

2741
auto outermostExpr = expr(unless(hasParent(expr())));
28-
auto leafStatement = stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
42+
auto leafStatement =
43+
stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
2944

3045
Finder->addMatcher(
3146
declRefExpr(
@@ -49,7 +64,7 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
4964
unless(hasDeclaration(
5065
varDecl(hasType(qualType(lValueReferenceType()))))),
5166

52-
hasAncestor(leafStatement.bind("leaf_statement")),
67+
hasAncestor(leafStatement.bind("leaf_statement")),
5368

5469
hasDeclaration(
5570
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
@@ -59,60 +74,44 @@ void LostStdMoveCheck::registerMatchers(MatchFinder *Finder) {
5974
this);
6075
}
6176

62-
const Expr *LostStdMoveCheck::getLastVarUsage(const VarDecl &Var,
63-
const Decl &Func,
64-
ASTContext &Context) {
65-
auto Exprs = allDeclRefExprs(Var, Func, Context);
66-
67-
const Expr *LastExpr = nullptr;
68-
for (const auto &Expr : Exprs) {
69-
if (!LastExpr)
70-
LastExpr = Expr;
71-
72-
if (LastExpr->getBeginLoc() < Expr->getBeginLoc())
73-
LastExpr = Expr;
74-
}
75-
76-
return LastExpr;
77-
}
78-
7977
template <typename Node>
8078
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
81-
llvm::SmallPtrSet<const Node *, 16> &Nodes) {
82-
for (const auto &Match : Matches)
79+
llvm::SmallPtrSet<const Node*, 16>& Nodes) {
80+
for (const BoundNodes& Match : Matches)
8381
Nodes.insert(Match.getNodeAs<Node>(ID));
8482
}
8583

86-
void LostStdMoveCheck::check(const MatchFinder::MatchResult &Result) {
87-
const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
88-
const auto *MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
89-
const auto *MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
90-
const auto *MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
91-
const auto *MatchedLeafStatement = Result.Nodes.getNodeAs<Stmt>("leaf_statement");
84+
void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
85+
const auto* MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
86+
const auto* MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
87+
const auto* MatchedUse = Result.Nodes.getNodeAs<Expr>("use");
88+
const auto* MatchedUseCall = Result.Nodes.getNodeAs<CallExpr>("use_parent");
89+
const auto* MatchedLeafStatement =
90+
Result.Nodes.getNodeAs<Stmt>("leaf_statement");
9291

93-
if (MatchedUseCall)
94-
return;
92+
if (MatchedUseCall) return;
9593

96-
const auto *LastUsage =
94+
const auto* LastUsage =
9795
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
98-
if (LastUsage == nullptr)
99-
return;
96+
if (LastUsage == nullptr) return;
10097

10198
if (LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
10299
// "use" is not the last reference to x
103100
return;
104101
}
105102

106103
// Calculate X usage count in the statement
107-
llvm::SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
108-
auto Matches = match(findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")), *MatchedLeafStatement, *Result.Context);
104+
llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
105+
ArrayRef<BoundNodes> Matches = match(
106+
findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")),
107+
*MatchedLeafStatement, *Result.Context);
109108
extractNodesByIdTo(Matches, "ref", DeclRefs);
110109
if (DeclRefs.size() > 1) {
111110
// Unspecified order of evaluation, e.g. f(x, x)
112111
return;
113112
}
114113

115-
diag(LastUsage->getBeginLoc(), "Could be std::move()");
114+
diag(LastUsage->getBeginLoc(), "could be std::move()");
116115
}
117116

118-
} // namespace clang::tidy::performance
117+
} // namespace clang::tidy::performance

clang-tools-extra/clang-tidy/performance/LostStdMoveCheck.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,8 @@ class LostStdMoveCheck : public ClangTidyCheck {
2424
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
2525
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
2626
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
27-
return LangOpts.CPlusPlus;
27+
return LangOpts.CPlusPlus11;
2828
}
29-
30-
private:
31-
const Expr *getLastVarUsage(const VarDecl &Var, const Decl &Func,
32-
ASTContext &Context);
3329
};
3430

3531
} // namespace clang::tidy::performance

0 commit comments

Comments
 (0)