Skip to content

Commit 4305993

Browse files
author
Shuai Wang
committedSep 17, 2018
[analyzer] Treat std::{move,forward} as casts in ExprMutationAnalyzer.
Summary: This is a follow up of D52008 and should make the analyzer being able to handle perfect forwardings in real world cases where forwardings are done through multiple layers of function calls with `std::forward`. Fixes PR38891. Reviewers: lebedev.ri, JonasToth, george.karpenkov Subscribers: xazax.hun, szepet, a.sidorin, mikhail.ramalho, Szelethus, cfe-commits Differential Revision: https://reviews.llvm.org/D52120 llvm-svn: 342409
1 parent 02bfd89 commit 4305993

File tree

2 files changed

+128
-25
lines changed

2 files changed

+128
-25
lines changed
 

‎clang/lib/Analysis/ExprMutationAnalyzer.cpp

+13-2
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,16 @@ const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) {
304304
nonConstReferenceType()))))
305305
.bind(NodeID<Expr>::value)),
306306
Stm, Context);
307-
return findExprMutation(Casts);
307+
if (const Stmt *S = findExprMutation(Casts))
308+
return S;
309+
// Treat std::{move,forward} as cast.
310+
const auto Calls =
311+
match(findAll(callExpr(callee(namedDecl(
312+
hasAnyName("::std::move", "::std::forward"))),
313+
hasArgument(0, equalsNode(Exp)))
314+
.bind("expr")),
315+
Stm, Context);
316+
return findExprMutation(Calls);
308317
}
309318

310319
const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) {
@@ -360,7 +369,9 @@ const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
360369
const auto IsInstantiated = hasDeclaration(isInstantiated());
361370
const auto FuncDecl = hasDeclaration(functionDecl().bind("func"));
362371
const auto Matches = match(
363-
findAll(expr(anyOf(callExpr(NonConstRefParam, IsInstantiated, FuncDecl),
372+
findAll(expr(anyOf(callExpr(NonConstRefParam, IsInstantiated, FuncDecl,
373+
unless(callee(namedDecl(hasAnyName(
374+
"::std::move", "::std::forward"))))),
364375
cxxConstructExpr(NonConstRefParam, IsInstantiated,
365376
FuncDecl)))
366377
.bind(NodeID<Expr>::value)),

‎clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

+115-23
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,25 @@ std::string removeSpace(std::string s) {
6666
return s;
6767
}
6868

69+
const std::string StdRemoveReference =
70+
"namespace std {"
71+
"template<class T> struct remove_reference { typedef T type; };"
72+
"template<class T> struct remove_reference<T&> { typedef T type; };"
73+
"template<class T> struct remove_reference<T&&> { typedef T type; }; }";
74+
75+
const std::string StdMove =
76+
"namespace std {"
77+
"template<class T> typename remove_reference<T>::type&& "
78+
"move(T&& t) noexcept {"
79+
"return static_cast<typename remove_reference<T>::type&&>(t); } }";
80+
81+
const std::string StdForward =
82+
"namespace std {"
83+
"template<class T> T&& "
84+
"forward(typename remove_reference<T>::type& t) noexcept { return t; }"
85+
"template<class T> T&& "
86+
"forward(typename remove_reference<T>::type&&) noexcept { return t; } }";
87+
6988
} // namespace
7089

7190
TEST(ExprMutationAnalyzerTest, Trivial) {
@@ -373,36 +392,87 @@ TEST(ExprMutationAnalyzerTest, ByConstRRefArgument) {
373392
}
374393

375394
TEST(ExprMutationAnalyzerTest, Move) {
376-
// Technically almost the same as ByNonConstRRefArgument, just double checking
377-
const auto AST = tooling::buildASTFromCode(
378-
"namespace std {"
379-
"template<class T> struct remove_reference { typedef T type; };"
380-
"template<class T> struct remove_reference<T&> { typedef T type; };"
381-
"template<class T> struct remove_reference<T&&> { typedef T type; };"
382-
"template<class T> typename std::remove_reference<T>::type&& "
383-
"move(T&& t) noexcept; }"
384-
"void f() { struct A {}; A x; std::move(x); }");
385-
const auto Results =
395+
auto AST =
396+
tooling::buildASTFromCode(StdRemoveReference + StdMove +
397+
"void f() { struct A {}; A x; std::move(x); }");
398+
auto Results =
386399
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
387-
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
400+
EXPECT_FALSE(isMutated(Results, AST.get()));
401+
402+
AST = tooling::buildASTFromCode(
403+
StdRemoveReference + StdMove +
404+
"void f() { struct A {}; A x, y; std::move(x) = y; }");
405+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
406+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x) = y"));
407+
408+
AST = tooling::buildASTFromCode(StdRemoveReference + StdMove +
409+
"void f() { int x, y; y = std::move(x); }");
410+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
411+
EXPECT_FALSE(isMutated(Results, AST.get()));
412+
413+
AST = tooling::buildASTFromCode(
414+
StdRemoveReference + StdMove +
415+
"struct S { S(); S(const S&); S& operator=(const S&); };"
416+
"void f() { S x, y; y = std::move(x); }");
417+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
418+
EXPECT_FALSE(isMutated(Results, AST.get()));
419+
420+
AST =
421+
tooling::buildASTFromCode(StdRemoveReference + StdMove +
422+
"struct S { S(); S(S&&); S& operator=(S&&); };"
423+
"void f() { S x, y; y = std::move(x); }");
424+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
425+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
426+
427+
AST =
428+
tooling::buildASTFromCode(StdRemoveReference + StdMove +
429+
"struct S { S(); S(const S&); S(S&&);"
430+
"S& operator=(const S&); S& operator=(S&&); };"
431+
"void f() { S x, y; y = std::move(x); }");
432+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
433+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
434+
435+
AST = tooling::buildASTFromCode(
436+
StdRemoveReference + StdMove +
437+
"struct S { S(); S(const S&); S(S&&);"
438+
"S& operator=(const S&); S& operator=(S&&); };"
439+
"void f() { const S x; S y; y = std::move(x); }");
440+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
441+
EXPECT_FALSE(isMutated(Results, AST.get()));
442+
443+
AST = tooling::buildASTFromCode(StdRemoveReference + StdMove +
444+
"struct S { S(); S(S); S& operator=(S); };"
445+
"void f() { S x, y; y = std::move(x); }");
446+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
447+
EXPECT_FALSE(isMutated(Results, AST.get()));
448+
449+
AST = tooling::buildASTFromCode(
450+
StdRemoveReference + StdMove +
451+
"struct S{}; void f() { S x, y; y = std::move(x); }");
452+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
453+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
454+
455+
AST = tooling::buildASTFromCode(
456+
StdRemoveReference + StdMove +
457+
"struct S{}; void f() { const S x; S y; y = std::move(x); }");
458+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
459+
EXPECT_FALSE(isMutated(Results, AST.get()));
388460
}
389461

390462
TEST(ExprMutationAnalyzerTest, Forward) {
391-
// Technically almost the same as ByNonConstRefArgument, just double checking
392-
const auto AST = tooling::buildASTFromCode(
393-
"namespace std {"
394-
"template<class T> struct remove_reference { typedef T type; };"
395-
"template<class T> struct remove_reference<T&> { typedef T type; };"
396-
"template<class T> struct remove_reference<T&&> { typedef T type; };"
397-
"template<class T> T&& "
398-
"forward(typename std::remove_reference<T>::type&) noexcept;"
399-
"template<class T> T&& "
400-
"forward(typename std::remove_reference<T>::type&&) noexcept;"
463+
auto AST = tooling::buildASTFromCode(
464+
StdRemoveReference + StdForward +
401465
"void f() { struct A {}; A x; std::forward<A &>(x); }");
402-
const auto Results =
466+
auto Results =
403467
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
468+
EXPECT_FALSE(isMutated(Results, AST.get()));
469+
470+
AST = tooling::buildASTFromCode(
471+
StdRemoveReference + StdForward +
472+
"void f() { struct A {}; A x, y; std::forward<A &>(x) = y; }");
473+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
404474
EXPECT_THAT(mutatedBy(Results, AST.get()),
405-
ElementsAre("std::forward<A &>(x)"));
475+
ElementsAre("std::forward<A &>(x) = y"));
406476
}
407477

408478
TEST(ExprMutationAnalyzerTest, CallUnresolved) {
@@ -639,6 +709,17 @@ TEST(ExprMutationAnalyzerTest, FollowFuncArgModified) {
639709
"void f() { int x; S s(x); }");
640710
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
641711
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
712+
713+
AST = tooling::buildASTFromCode(
714+
StdRemoveReference + StdForward +
715+
"template <class... Args> void u(Args&...);"
716+
"template <class... Args> void h(Args&&... args)"
717+
"{ u(std::forward<Args>(args)...); }"
718+
"template <class... Args> void g(Args&&... args)"
719+
"{ h(std::forward<Args>(args)...); }"
720+
"void f() { int x; g(x); }");
721+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
722+
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
642723
}
643724

644725
TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
@@ -683,6 +764,17 @@ TEST(ExprMutationAnalyzerTest, FollowFuncArgNotModified) {
683764
"void f() { int x; S s(x); }");
684765
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
685766
EXPECT_FALSE(isMutated(Results, AST.get()));
767+
768+
AST = tooling::buildASTFromCode(
769+
StdRemoveReference + StdForward +
770+
"template <class... Args> void u(Args...);"
771+
"template <class... Args> void h(Args&&... args)"
772+
"{ u(std::forward<Args>(args)...); }"
773+
"template <class... Args> void g(Args&&... args)"
774+
"{ h(std::forward<Args>(args)...); }"
775+
"void f() { int x; g(x); }");
776+
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
777+
EXPECT_FALSE(isMutated(Results, AST.get()));
686778
}
687779

688780
TEST(ExprMutationAnalyzerTest, ArrayElementModified) {

0 commit comments

Comments
 (0)
Please sign in to comment.