r355605 - [analyzer] handle modification of vars inside an expr with comma operator
Petar Jovanovic via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 7 07:50:52 PST 2019
Author: petarj
Date: Thu Mar 7 07:50:52 2019
New Revision: 355605
URL: http://llvm.org/viewvc/llvm-project?rev=355605&view=rev
Log:
[analyzer] handle modification of vars inside an expr with comma operator
We should track mutation of a variable within a comma operator expression.
Current code in ExprMutationAnalyzer does not handle it.
This will handle cases like:
(a, b) ++ < == b is modified
(a, b) = c < == b is modifed
Patch by Djordje Todorovic.
Differential Revision: https://reviews.llvm.org/D58894
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=355605&r1=355604&r2=355605&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Mar 7 07:50:52 2019
@@ -3403,6 +3403,9 @@ public:
static bool isComparisonOp(Opcode Opc) { return Opc >= BO_Cmp && Opc<=BO_NE; }
bool isComparisonOp() const { return isComparisonOp(getOpcode()); }
+ static bool isCommaOp(Opcode Opc) { return Opc == BO_Comma; }
+ bool isCommaOp() const { return isCommaOp(getOpcode()); }
+
static Opcode negateComparisonOp(Opcode Opc) {
switch (Opc) {
default:
Modified: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp?rev=355605&r1=355604&r2=355605&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp (original)
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp Thu Mar 7 07:50:52 2019
@@ -24,6 +24,18 @@ AST_MATCHER_P(CXXForRangeStmt, hasRangeS
return InnerMatcher.matches(*Range, Finder, Builder);
}
+AST_MATCHER_P(Expr, maybeEvalCommaExpr,
+ ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+ const Expr* Result = &Node;
+ while (const auto *BOComma =
+ dyn_cast_or_null<BinaryOperator>(Result->IgnoreParens())) {
+ if (!BOComma->isCommaOp())
+ break;
+ Result = BOComma->getRHS();
+ }
+ return InnerMatcher.matches(*Result, Finder, Builder);
+}
+
const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt, CXXTypeidExpr>
cxxTypeidExpr;
@@ -193,24 +205,28 @@ const Stmt *ExprMutationAnalyzer::findDe
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
const auto AsAssignmentLhs =
- binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));
+ binaryOperator(isAssignmentOperator(),
+ hasLHS(maybeEvalCommaExpr(equalsNode(Exp))));
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
- hasUnaryOperand(equalsNode(Exp)));
+ hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp))));
// Invoking non-const member function.
// A member function is assumed to be non-const when it is unresolved.
const auto NonConstMethod = cxxMethodDecl(unless(isConst()));
const auto AsNonConstThis =
- expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(Exp))),
+ expr(anyOf(cxxMemberCallExpr(callee(NonConstMethod),
+ on(maybeEvalCommaExpr(equalsNode(Exp)))),
cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0, equalsNode(Exp))),
+ hasArgument(0,
+ maybeEvalCommaExpr(equalsNode(Exp)))),
callExpr(callee(expr(anyOf(
- unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))),
+ unresolvedMemberExpr(
+ hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp)))),
cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp)))))))));
+ hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp))))))))));
// Taking address of 'Exp'.
// We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -220,10 +236,11 @@ const Stmt *ExprMutationAnalyzer::findDi
unaryOperator(hasOperatorName("&"),
// A NoOp implicit cast is adding const.
unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp)))),
- hasUnaryOperand(equalsNode(Exp)));
+ hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp))));
const auto AsPointerFromArrayDecay =
castExpr(hasCastKind(CK_ArrayToPointerDecay),
- unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp)));
+ unless(hasParent(arraySubscriptExpr())),
+ has(maybeEvalCommaExpr(equalsNode(Exp))));
// Treat calling `operator->()` of move-only classes as taking address.
// These are typically smart pointers with unique ownership so we treat
// mutation of pointee as mutation of the smart pointer itself.
@@ -231,7 +248,8 @@ const Stmt *ExprMutationAnalyzer::findDi
cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
callee(cxxMethodDecl(ofClass(isMoveOnly()),
returns(nonConstPointerType()))),
- argumentCountIs(1), hasArgument(0, equalsNode(Exp)));
+ argumentCountIs(1),
+ hasArgument(0, maybeEvalCommaExpr(equalsNode(Exp))));
// Used as non-const-ref argument when calling a function.
// An argument is assumed to be non-const-ref when the function is unresolved.
@@ -239,7 +257,8 @@ const Stmt *ExprMutationAnalyzer::findDi
// findFunctionArgMutation which has additional smarts for handling forwarding
// references.
const auto NonConstRefParam = forEachArgumentWithParam(
- equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType())));
+ maybeEvalCommaExpr(equalsNode(Exp)),
+ parmVarDecl(hasType(nonConstReferenceType())));
const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
const auto AsNonConstRefArg = anyOf(
callExpr(NonConstRefParam, NotInstantiated),
@@ -247,8 +266,8 @@ const Stmt *ExprMutationAnalyzer::findDi
callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
cxxDependentScopeMemberExpr(),
hasType(templateTypeParmType())))),
- hasAnyArgument(equalsNode(Exp))),
- cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp))));
+ hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))),
+ cxxUnresolvedConstructExpr(hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))));
// Captured by a lambda by reference.
// If we're initializing a capture with 'Exp' directly then we're initializing
@@ -261,7 +280,8 @@ const Stmt *ExprMutationAnalyzer::findDi
// For returning by value there will be an ImplicitCastExpr <LValueToRValue>.
// For returning by const-ref there will be an ImplicitCastExpr <NoOp> (for
// adding const.)
- const auto AsNonConstRefReturn = returnStmt(hasReturnValue(equalsNode(Exp)));
+ const auto AsNonConstRefReturn = returnStmt(hasReturnValue(
+ maybeEvalCommaExpr(equalsNode(Exp))));
const auto Matches =
match(findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, AsNonConstThis,
Modified: cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp?rev=355605&r1=355604&r2=355605&view=diff
==============================================================================
--- cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp (original)
+++ cfe/trunk/unittests/Analysis/ExprMutationAnalyzerTest.cpp Thu Mar 7 07:50:52 2019
@@ -881,6 +881,137 @@ TEST(ExprMutationAnalyzerTest, CastToCon
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, CommaExprWithAnAssigment) {
+ const auto AST =
+ buildASTFromCodeWithArgs("void f() { int x; int y; (x, y) = 5; }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithDecOp) {
+ const auto AST =
+ buildASTFromCodeWithArgs("void f() { int x; int y; (x, y)++; }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithNonConstMemberCall) {
+ const auto AST =
+ buildASTFromCodeWithArgs("class A { public: int mem; void f() { mem ++; } };"
+ "void fn() { A o1, o2; (o1, o2).f(); }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("o2")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithConstMemberCall) {
+ const auto AST =
+ buildASTFromCodeWithArgs("class A { public: int mem; void f() const { } };"
+ "void fn() { A o1, o2; (o1, o2).f(); }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("o2")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithCallExpr) {
+ const auto AST =
+ buildASTFromCodeWithArgs("class A { public: int mem; void f(A &O1) {} };"
+ "void fn() { A o1, o2; o2.f((o2, o1)); }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("o1")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithCallUnresolved) {
+ auto AST = buildASTFromCodeWithArgs(
+ "template <class T> struct S;"
+ "template <class T> void f() { S<T> s; int x, y; s.mf((y, x)); }",
+ {"-fno-delayed-template-parsing -Wno-unused-value"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+
+ AST = buildASTFromCodeWithArgs(
+ "template <class T> void f(T t) { int x, y; g(t, (y, x)); }",
+ {"-fno-delayed-template-parsing -Wno-unused-value"});
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprParmRef) {
+ const auto AST =
+ buildASTFromCodeWithArgs("class A { public: int mem;};"
+ "extern void fn(A &o1);"
+ "void fn2 () { A o1, o2; fn((o2, o1)); } ",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("o1")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprWithAmpersandOp) {
+ const auto AST =
+ buildASTFromCodeWithArgs("class A { public: int mem;};"
+ "void fn () { A o1, o2;"
+ "void *addr = &(o2, o1); } ",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("o1")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprAsReturnAsValue) {
+ auto AST = buildASTFromCodeWithArgs("int f() { int x, y; return (x, y); }",
+ {"-Wno-unused-value"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaEpxrAsReturnAsNonConstRef) {
+ const auto AST =
+ buildASTFromCodeWithArgs("int& f() { int x, y; return (y, x); }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprAsArrayToPointerDecay) {
+ const auto AST =
+ buildASTFromCodeWithArgs("void g(int*); "
+ "void f() { int x[2], y[2]; g((y, x)); }",
+ {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+TEST(ExprMutationAnalyzerTest, CommaExprAsUniquePtr) {
+ const std::string UniquePtrDef =
+ "template <class T> struct UniquePtr {"
+ " UniquePtr();"
+ " UniquePtr(const UniquePtr&) = delete;"
+ " T& operator*() const;"
+ " T* operator->() const;"
+ "};";
+ const auto AST = buildASTFromCodeWithArgs(
+ UniquePtrDef + "template <class T> void f() "
+ "{ UniquePtr<T> x; UniquePtr<T> y;"
+ " (y, x)->mf(); }",
+ {"-fno-delayed-template-parsing -Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByValue) {
const auto AST = buildASTFromCode("void f() { int x; [=]() { x; }; }");
const auto Results =
More information about the cfe-commits
mailing list