Skip to content

Commit 1710c2f

Browse files
committed
w
1 parent 592d79b commit 1710c2f

File tree

1 file changed

+64
-25
lines changed

1 file changed

+64
-25
lines changed

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

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,40 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "LostStdMoveCheck.h"
10-
#include "../utils/DeclRefExprUtils.h"
1110
#include "clang/ASTMatchers/ASTMatchFinder.h"
1211
#include "clang/Lex/Lexer.h"
1312

1413
using namespace clang::ast_matchers;
1514

1615
namespace clang::tidy::performance {
1716

18-
using utils::decl_ref_expr::allDeclRefExprs;
17+
template <typename Node>
18+
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
19+
llvm::SmallPtrSet<const Node*, 16>& Nodes) {
20+
for (const BoundNodes& Match : Matches)
21+
Nodes.insert(Match.getNodeAs<Node>(ID));
22+
}
23+
24+
llvm::SmallPtrSet<const DeclRefExpr*, 16> allDeclRefExprsHonourLambda(
25+
const VarDecl& VarDecl, const Decl& Decl, ASTContext& Context) {
26+
auto Matches = match(
27+
decl(forEachDescendant(
28+
declRefExpr(to(varDecl(equalsNode(&VarDecl))),
29+
30+
unless(hasAncestor(lambdaExpr(hasAnyCapture(lambdaCapture(
31+
capturesVar(varDecl(equalsNode(&VarDecl))))))))
32+
33+
)
34+
.bind("declRef"))),
35+
Decl, Context);
36+
llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
37+
extractNodesByIdTo(Matches, "declRef", DeclRefs);
38+
return DeclRefs;
39+
}
1940

2041
static const Expr* getLastVarUsage(const VarDecl& Var, const Decl& Func,
2142
ASTContext& Context) {
22-
auto Exprs = allDeclRefExprs(Var, Func, Context);
43+
auto Exprs = allDeclRefExprsHonourLambda(Var, Func, Context);
2344

2445
const Expr* LastExpr = nullptr;
2546
for (const clang::DeclRefExpr* Expr : Exprs) {
@@ -36,17 +57,16 @@ AST_MATCHER(CXXRecordDecl, hasTrivialMoveConstructor) {
3657
}
3758

3859
void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
39-
auto returnParent =
60+
auto ReturnParent =
4061
hasParent(expr(hasParent(cxxConstructExpr(hasParent(returnStmt())))));
4162

42-
auto outermostExpr = expr(unless(hasParent(expr())));
43-
auto leafStatement =
44-
stmt(outermostExpr, unless(hasDescendant(outermostExpr)));
63+
auto OutermostExpr = expr(unless(hasParent(expr())));
64+
auto LeafStatement = stmt(OutermostExpr);
4565

4666
Finder->addMatcher(
4767
declRefExpr(
4868
// not "return x;"
49-
unless(returnParent),
69+
unless(ReturnParent),
5070

5171
unless(hasType(namedDecl(hasName("::std::string_view")))),
5272

@@ -58,14 +78,17 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
5878
hasDeclaration(cxxRecordDecl(hasTrivialMoveConstructor()))))),
5979

6080
// Not in a cycle
61-
unless(hasAncestor(forStmt())), unless(hasAncestor(doStmt())),
81+
unless(hasAncestor(forStmt())),
82+
83+
unless(hasAncestor(doStmt())),
84+
6285
unless(hasAncestor(whileStmt())),
6386

6487
// only non-X&
6588
unless(hasDeclaration(
6689
varDecl(hasType(qualType(lValueReferenceType()))))),
6790

68-
hasAncestor(leafStatement.bind("leaf_statement")),
91+
hasAncestor(LeafStatement.bind("leaf_statement")),
6992

7093
hasDeclaration(
7194
varDecl(hasAncestor(functionDecl().bind("func"))).bind("decl")),
@@ -86,13 +109,6 @@ void LostStdMoveCheck::registerMatchers(MatchFinder* Finder) {
86109
this);
87110
}
88111

89-
template <typename Node>
90-
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
91-
llvm::SmallPtrSet<const Node*, 16>& Nodes) {
92-
for (const BoundNodes& Match : Matches)
93-
Nodes.insert(Match.getNodeAs<Node>(ID));
94-
}
95-
96112
void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
97113
const auto* MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("decl");
98114
const auto* MatchedFunc = Result.Nodes.getNodeAs<FunctionDecl>("func");
@@ -101,21 +117,35 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
101117
const auto* MatchedLeafStatement =
102118
Result.Nodes.getNodeAs<Stmt>("leaf_statement");
103119

104-
if (MatchedUseCall) return;
120+
if (MatchedUseCall) {
121+
return;
122+
}
105123

106124
const Expr* LastUsage =
107125
getLastVarUsage(*MatchedDecl, *MatchedFunc, *Result.Context);
108-
if (LastUsage == nullptr) return;
109126

110-
if (LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
127+
if (LastUsage && LastUsage->getBeginLoc() > MatchedUse->getBeginLoc()) {
111128
// "use" is not the last reference to x
112129
return;
113130
}
114131

132+
if (LastUsage &&
133+
LastUsage->getSourceRange() != MatchedUse->getSourceRange()) {
134+
return;
135+
}
136+
115137
// Calculate X usage count in the statement
116138
llvm::SmallPtrSet<const DeclRefExpr*, 16> DeclRefs;
117139
ArrayRef<BoundNodes> Matches = match(
118-
findAll(declRefExpr(to(varDecl(equalsNode(MatchedDecl)))).bind("ref")),
140+
findAll(declRefExpr(
141+
142+
to(varDecl(equalsNode(MatchedDecl))),
143+
144+
unless(hasAncestor(lambdaExpr(hasAnyCapture(lambdaCapture(
145+
capturesVar(varDecl(equalsNode(MatchedDecl))))))))
146+
147+
)
148+
.bind("ref")),
119149
*MatchedLeafStatement, *Result.Context);
120150
extractNodesByIdTo(Matches, "ref", DeclRefs);
121151
if (DeclRefs.size() > 1) {
@@ -125,12 +155,21 @@ void LostStdMoveCheck::check(const MatchFinder::MatchResult& Result) {
125155

126156
const SourceManager& Source = Result.Context->getSourceManager();
127157
const auto Range =
128-
CharSourceRange::getTokenRange(LastUsage->getSourceRange());
158+
CharSourceRange::getTokenRange(MatchedUse->getSourceRange());
129159
const StringRef NeedleExprCode =
130160
Lexer::getSourceText(Range, Source, Result.Context->getLangOpts());
131-
diag(LastUsage->getBeginLoc(), "could be std::move()")
132-
<< FixItHint::CreateReplacement(
133-
Range, ("std::move(" + NeedleExprCode + ")").str());
161+
162+
if (NeedleExprCode == "=") {
163+
164+
diag(MatchedUse->getBeginLoc(), "could be std::move()")
165+
<< FixItHint::CreateInsertion(
166+
MatchedUse->getBeginLoc(),
167+
("std::move(" + MatchedDecl->getName() + "),").str());
168+
} else {
169+
diag(MatchedUse->getBeginLoc(), "could be std::move()")
170+
<< FixItHint::CreateReplacement(
171+
Range, ("std::move(" + NeedleExprCode + ")").str());
172+
}
134173
}
135174

136175
} // namespace clang::tidy::performance

0 commit comments

Comments
 (0)