JonasToth updated this revision to Diff 296546.
JonasToth added a comment.
Herald added a subscriber: steakhal.
- document sections in the testcases, hopefully little more structure for
comprehension
- fix derived-to-base-cast OUTSIDE of an conditional operator
- improve type-dependent callexpr, too
- better canResolveToExpr matcher code
- adjust matcher for comma
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88088/new/
https://reviews.llvm.org/D88088
Files:
clang/lib/Analysis/ExprMutationAnalyzer.cpp
clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===================================================================
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -19,9 +19,7 @@
using namespace clang::ast_matchers;
using ::testing::ElementsAre;
-using ::testing::IsEmpty;
using ::testing::ResultOf;
-using ::testing::StartsWith;
using ::testing::Values;
namespace {
@@ -63,12 +61,16 @@
const auto *const S = selectFirst<Stmt>("stmt", Results);
SmallVector<std::string, 1> Chain;
ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
for (const auto *E = selectFirst<Expr>("expr", Results); E != nullptr;) {
const Stmt *By = Analyzer.findMutation(E);
- std::string buffer;
- llvm::raw_string_ostream stream(buffer);
- By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
- Chain.push_back(StringRef(stream.str()).trim().str());
+ if (!By)
+ break;
+
+ std::string Buffer;
+ llvm::raw_string_ostream Stream(Buffer);
+ By->printPretty(Stream, nullptr, AST->getASTContext().getPrintingPolicy());
+ Chain.emplace_back(StringRef(Stream.str()).trim().str());
E = dyn_cast<DeclRefExpr>(By);
}
return Chain;
@@ -111,7 +113,13 @@
class AssignmentTest : public ::testing::TestWithParam<std::string> {};
+// This test is for the most basic and direct modification of a variable,
+// assignment to it (e.g. `x = 10;`).
+// It additionally tests, that reference to a variable are not only captured
+// directly, but expression that result in the variable are handled, too.
+// This includes the comma operator, parens and the ternary operator.
TEST_P(AssignmentTest, AssignmentModifies) {
+ // Test the detection of the raw expression modifications.
{
const std::string ModExpr = "x " + GetParam() + " 10";
const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -120,6 +128,7 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
}
+ // Test the detection if the expression is surrounded by parens.
{
const std::string ModExpr = "(x) " + GetParam() + " 10";
const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -127,12 +136,79 @@
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
}
+
+ // Test the detection if the comma operator yields the expression as result.
+ {
+ const std::string ModExpr = "x " + GetParam() + " 10";
+ const auto AST = buildASTFromCodeWithArgs(
+ "void f() { int x, y; y, " + ModExpr + "; }", {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Ensure no detection if t he comma operator does not yield the expression as
+ // result.
+ {
+ const std::string ModExpr = "y, x, y " + GetParam() + " 10";
+ const auto AST = buildASTFromCodeWithArgs(
+ "void f() { int x, y; " + ModExpr + "; }", {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression.
+ {
+ const std::string ModExpr = "(y != 0 ? y : x) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression
+ // through multiple nesting of ternary operators.
+ {
+ const std::string ModExpr =
+ "(y != 0 ? (y > 5 ? y : x) : (y)) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
+
+ // Test the detection if the a ternary operator can result in the expression
+ // with additional parens.
+ {
+ const std::string ModExpr = "(y != 0 ? (y) : ((x))) " + GetParam() + " 10";
+ const auto AST =
+ buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+ }
}
INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
"^=", "<<=", ">>="), );
+TEST(ExprMutationAnalyzerTest, AssignmentConditionalWithInheritance) {
+ const auto AST = buildASTFromCode("struct Base {void nonconst(); };"
+ "struct Derived : Base {};"
+ "static void f() {"
+ " Derived x, y;"
+ " Base &b = true ? x : y;"
+ " b.nonconst();"
+ "}");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("b", "b.nonconst()"));
+}
+
class IncDecTest : public ::testing::TestWithParam<std::string> {};
TEST_P(IncDecTest, IncDecModifies) {
@@ -147,6 +223,8 @@
Values("++x", "--x", "x++", "x--", "++(x)", "--(x)",
"(x)++", "(x)--"), );
+// Section: member functions
+
TEST(ExprMutationAnalyzerTest, NonConstMemberFunc) {
const auto AST = buildASTFromCode(
"void f() { struct Foo { void mf(); }; Foo x; x.mf(); }");
@@ -185,6 +263,18 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, TypeDependentMemberCall) {
+ const auto AST = buildASTFromCodeWithArgs(
+ "template <class T> class vector { void push_back(T); }; "
+ "template <class T> void f() { vector<T> x; x.push_back(T()); }",
+ {"-fno-delayed-template-parsing"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.push_back(T())"));
+}
+
+// Section: overloaded operators
+
TEST(ExprMutationAnalyzerTest, NonConstOperator) {
const auto AST = buildASTFromCode(
"void f() { struct Foo { Foo& operator=(int); }; Foo x; x = 10; }");
@@ -201,6 +291,19 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
+ const auto AST = buildASTFromCodeWithArgs(
+ "template <typename Stream> void input_operator_template() {"
+ "Stream x; unsigned y = 42;"
+ "x >> y; }",
+ {"-fno-delayed-template-parsing"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+}
+
+// Section: expression as call argument
+
TEST(ExprMutationAnalyzerTest, ByValueArgument) {
auto AST = buildASTFromCode("void g(int); void f() { int x; g(x); }");
auto Results =
@@ -322,6 +425,48 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("A<0>::mf(x)"));
}
+TEST(ExprMutationAnalyzerTest, ByNonConstRefArgumentFunctionTypeDependent) {
+#if 1
+ // This testcase did not actually reproduce a problem. Maybe the second one
+ // points to the same issue!
+ auto AST = buildASTFromCode(
+ "template <typename CBTy> static void foreachUse(CBTy CB) {"
+ " int array[4] = {1, 2, 3, 4};"
+ " for (unsigned idx = 0; idx < 4; ++idx) {"
+ " int &x = array[idx];"
+ " CB(x);"
+ " }"
+ "}"
+ "void usage1() {"
+ " auto const_lambda = [](int arg) { (void) arg; };"
+ " foreachUse(const_lambda);"
+ "}"
+ "void usage2() {"
+ " int number = 42;"
+ " auto mod_lambda = [&](int& arg) { arg+= number; };"
+ " foreachUse(mod_lambda);"
+ "}");
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("CB(x)"));
+#endif
+
+#if 1
+ AST = buildASTFromCodeWithArgs(
+ "enum MyEnum { foo, bar };"
+ "void tryParser(unsigned& first, MyEnum Type) { first++, (void)Type; }"
+ "template <MyEnum Type> void parse() {"
+ " auto parser = [](unsigned& first) { first++; tryParser(first, Type); };"
+ " unsigned x = 42;"
+ " parser(x);"
+ "}",
+ {"-fno-delayed-template-parsing"});
+ Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("parser(x)"));
+#endif
+}
+
TEST(ExprMutationAnalyzerTest, ByConstRefArgument) {
auto AST = buildASTFromCode("void g(const int&); void f() { int x; g(x); }");
auto Results =
@@ -394,24 +539,30 @@
"void g(const int&&); void f() { int x; g(static_cast<int&&>(x)); }");
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);"
"void f() { A x; static_cast<A&&>(x) + 1; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<A &&>(x)"));
AST = buildASTFromCode("void f() { struct A { A(const int&&); }; "
"int x; A y(static_cast<int&&>(x)); }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
AST = buildASTFromCode("void f() { struct A { A(); A(const A&&); }; "
"A x; A y(static_cast<A&&>(x)); }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<A &&>(x)"));
}
+// section: explicit std::move and std::forward testing
+
TEST(ExprMutationAnalyzerTest, Move) {
auto AST = buildASTFromCode(StdRemoveReference + StdMove +
"void f() { struct A {}; A x; std::move(x); }");
@@ -490,6 +641,9 @@
ElementsAre("std::forward<A &>(x) = y"));
}
+// section: template constellations that prohibit reasoning about modifications
+// as it depends on instantiations.
+
TEST(ExprMutationAnalyzerTest, CallUnresolved) {
auto AST =
buildASTFromCodeWithArgs("template <class T> void f() { T x; g(x); }",
@@ -543,6 +697,8 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("T(x)"));
}
+// section: return values
+
TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
auto AST = buildASTFromCode("int f() { int x; return x; }");
auto Results =
@@ -579,7 +735,7 @@
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("return static_cast<int &&>(x);"));
+ ElementsAre("static_cast<int &&>(x)"));
}
TEST(ExprMutationAnalyzerTest, ReturnAsConstRRef) {
@@ -587,9 +743,12 @@
"const int&& f() { int x; return static_cast<int&&>(x); }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &&>(x)"));
}
+// section: taking the address of a variable and pointers
+
TEST(ExprMutationAnalyzerTest, TakeAddress) {
const auto AST = buildASTFromCode("void g(int*); void f() { int x; g(&x); }");
const auto Results =
@@ -621,6 +780,9 @@
EXPECT_THAT(mutatedBy(ResultsY, AST.get()), ElementsAre("y"));
}
+// section: special case: all created references are non-mutating themself
+// and therefore all become 'const'/the value is not modified!
+
TEST(ExprMutationAnalyzerTest, FollowRefModified) {
auto AST = buildASTFromCode(
"void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
@@ -792,6 +954,8 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: builtin arrays
+
TEST(ExprMutationAnalyzerTest, ArrayElementModified) {
const auto AST = buildASTFromCode("void f() { int x[2]; x[0] = 10; }");
const auto Results =
@@ -806,6 +970,8 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: member modifications
+
TEST(ExprMutationAnalyzerTest, NestedMemberModified) {
auto AST =
buildASTFromCode("void f() { struct A { int vi; }; struct B { A va; }; "
@@ -849,6 +1015,8 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: casts
+
TEST(ExprMutationAnalyzerTest, CastToValue) {
const auto AST =
buildASTFromCode("void f() { int x; static_cast<double>(x); }");
@@ -863,13 +1031,13 @@
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("static_cast<int &>(x) = 10"));
+ ElementsAre("static_cast<int &>(x)"));
AST = buildASTFromCode("typedef int& IntRef;"
"void f() { int x; static_cast<IntRef>(x) = 10; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
EXPECT_THAT(mutatedBy(Results, AST.get()),
- ElementsAre("static_cast<IntRef>(x) = 10"));
+ ElementsAre("static_cast<IntRef>(x)"));
}
TEST(ExprMutationAnalyzerTest, CastToRefNotModified) {
@@ -877,7 +1045,8 @@
buildASTFromCode("void f() { int x; static_cast<int&>(x); }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("static_cast<int &>(x)"));
}
TEST(ExprMutationAnalyzerTest, CastToConstRef) {
@@ -893,6 +1062,8 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: comma expressions
+
TEST(ExprMutationAnalyzerTest, CommaExprWithAnAssigment) {
const auto AST = buildASTFromCodeWithArgs(
"void f() { int x; int y; (x, y) = 5; }", {"-Wno-unused-value"});
@@ -1019,6 +1190,18 @@
EXPECT_TRUE(isMutated(Results, AST.get()));
}
+TEST(ExprMutationAnalyzerTest, CommaNestedConditional) {
+ const std::string Code = "void f() { int x, y = 42;"
+ " y, (true ? x : y) = 42; }";
+ const auto AST = buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("(true ? x : y) = 42"));
+}
+
+// section: lambda captures
+
TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByValue) {
const auto AST = buildASTFromCode("void f() { int x; [=]() { x; }; }");
const auto Results =
@@ -1049,25 +1232,29 @@
ElementsAre(ResultOf(removeSpace, "[&x](){x=10;}")));
}
+// section: range-for loops
+
TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModified) {
auto AST =
buildASTFromCode("void f() { int x[2]; for (int& e : x) e = 10; }");
auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (int &e : x)\n e = 10;"));
AST = buildASTFromCode("typedef int& IntRef;"
"void f() { int x[2]; for (IntRef e : x) e = 10; }");
Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (IntRef e : x)\n e = 10;"));
}
-TEST(ExprMutationAnalyzerTest, RangeForArrayByRefNotModified) {
+TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModifiedByImplicitInit) {
const auto AST =
buildASTFromCode("void f() { int x[2]; for (int& e : x) e; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_TRUE(isMutated(Results, AST.get()));
}
TEST(ExprMutationAnalyzerTest, RangeForArrayByValue) {
@@ -1107,7 +1294,8 @@
"void f() { V x; for (int& e : x) e = 10; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+ EXPECT_THAT(mutatedBy(Results, AST.get()),
+ ElementsAre("for (int &e : x)\n e = 10;"));
}
TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefNotModified) {
@@ -1115,7 +1303,7 @@
"void f() { V x; for (int& e : x) e; }");
const auto Results =
match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
- EXPECT_FALSE(isMutated(Results, AST.get()));
+ EXPECT_TRUE(isMutated(Results, AST.get()));
}
TEST(ExprMutationAnalyzerTest, RangeForNonArrayByValue) {
@@ -1136,6 +1324,8 @@
EXPECT_FALSE(isMutated(Results, AST.get()));
}
+// section: unevaluated expressions
+
TEST(ExprMutationAnalyzerTest, UnevaluatedExpressions) {
auto AST = buildASTFromCode("void f() { int x, y; decltype(x = 10) z = y; }");
auto Results =
@@ -1189,6 +1379,8 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()"));
}
+// section: special case: smartpointers
+
TEST(ExprMutationAnalyzerTest, UniquePtr) {
const std::string UniquePtrDef =
"template <class T> struct UniquePtr {"
@@ -1246,6 +1438,24 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x->mf()"));
}
+// section: complex problems detected on real code
+
+TEST(ExprMutationAnalyzerTest, UnevaluatedContext) {
+ const std::string Example =
+ "template <typename T>"
+ "struct to_construct : T { to_construct(int &j) {} };"
+ "template <typename T>"
+ "void placement_new_in_unique_ptr() { int x = 0;"
+ " new to_construct<T>(x);"
+ "}";
+ auto AST =
+ buildASTFromCodeWithArgs(Example, {"-fno-delayed-template-parsing"});
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_TRUE(isMutated(Results, AST.get()));
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("(x)"));
+}
+
TEST(ExprMutationAnalyzerTest, ReproduceFailureMinimal) {
const std::string Reproducer =
"namespace std {"
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===================================================================
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -6,7 +6,10 @@
//
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/OperationKinds.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/STLExtras.h"
namespace clang {
@@ -24,11 +27,11 @@
return InnerMatcher.matches(*Range, Finder, Builder);
}
-AST_MATCHER_P(Expr, maybeEvalCommaExpr,
- ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
- const Expr* Result = &Node;
+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())) {
+ dyn_cast_or_null<BinaryOperator>(Result->IgnoreParens())) {
if (!BOComma->isCommaOp())
break;
Result = BOComma->getRHS();
@@ -36,6 +39,52 @@
return InnerMatcher.matches(*Result, Finder, Builder);
}
+AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ auto DerivedToBase = [](const ast_matchers::internal::Matcher<Expr> &Inner) {
+ return implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase),
+ hasCastKind(CK_UncheckedDerivedToBase)),
+ hasSourceExpression(Inner));
+ };
+ // Matches unless the value is a derived class and is assigned to a
+ // reference to the base class. Other implicit casts should not happen.
+ auto IgnoreDerivedToBase =
+ [&DerivedToBase](const ast_matchers::internal::Matcher<Expr> &Inner) {
+ return ignoringParens(expr(anyOf(Inner, DerivedToBase(Inner))));
+ };
+
+ // The 'ConditionalOperator' matches on `<anything> ? <expr> : <expr>`.
+ // This matching must be recursive, because <expr> can be anything resolving
+ // to the `InnerMatcher`, for example another conditional operator.
+ // The edge-case `BaseClass &b = <cond> ? DerivedVar1 : DerivedVar2;`
+ // is handled, too. The implicit cast happens outside of the conditional.
+ // This is matched by `DerivedToBase(canResolveToExpr(InnerMatcher))` below.
+ auto const ConditionalOperator = conditionalOperator(anyOf(
+ hasTrueExpression(ignoringParens(canResolveToExpr(InnerMatcher))),
+ hasFalseExpression(ignoringParens(canResolveToExpr(InnerMatcher)))));
+
+ auto const ComplexMatcher = ignoringParens(
+ expr(anyOf(IgnoreDerivedToBase(InnerMatcher),
+ maybeEvalCommaExpr(IgnoreDerivedToBase(InnerMatcher)),
+ IgnoreDerivedToBase(ConditionalOperator))));
+
+ return ComplexMatcher.matches(Node, Finder, Builder);
+}
+
+// Similar to 'hasAnyArgument', but does not work because 'InitListExpr' does
+// not have the 'arguments()' method.
+AST_MATCHER_P(InitListExpr, hasAnyInit, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ for (const Expr *Arg : Node.inits()) {
+ ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
+ if (InnerMatcher.matches(*Arg, Finder, &Result)) {
+ *Builder = std::move(Result);
+ return true;
+ }
+ }
+ return false;
+}
+
const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt, CXXTypeidExpr>
cxxTypeidExpr;
@@ -151,7 +200,7 @@
NodeID<Expr>::value,
match(
findAll(
- expr(equalsNode(Exp),
+ expr(canResolveToExpr(equalsNode(Exp)),
anyOf(
// `Exp` is part of the underlying expression of
// decltype/typeof if it has an ancestor of
@@ -202,29 +251,43 @@
const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) {
// LHS of any assignment operators.
const auto AsAssignmentLhs = binaryOperator(
- isAssignmentOperator(),
- hasLHS(maybeEvalCommaExpr(ignoringParenImpCasts(equalsNode(Exp)))));
+ isAssignmentOperator(), hasLHS(canResolveToExpr(equalsNode(Exp))));
// Operand of increment/decrement operators.
const auto AsIncDecOperand =
unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")),
- hasUnaryOperand(maybeEvalCommaExpr(
- ignoringParenImpCasts(equalsNode(Exp)))));
+ hasUnaryOperand(canResolveToExpr(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(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxOperatorCallExpr(callee(NonConstMethod),
- hasArgument(0,
- maybeEvalCommaExpr(equalsNode(Exp)))),
- callExpr(callee(expr(anyOf(
- unresolvedMemberExpr(
- hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxDependentScopeMemberExpr(
- hasObjectExpression(maybeEvalCommaExpr(equalsNode(Exp))))))))));
+
+ const auto AsNonConstThis = expr(anyOf(
+ cxxMemberCallExpr(callee(NonConstMethod),
+ on(canResolveToExpr(equalsNode(Exp)))),
+ cxxOperatorCallExpr(callee(NonConstMethod),
+ hasArgument(0, canResolveToExpr(equalsNode(Exp)))),
+ // In case of a templated type, calling overloaded operators is not
+ // resolved and modelled as `binaryOperator` on a dependent type.
+ // Such instances are considered a modification, because they can modify
+ // in different instantiations of the template.
+ binaryOperator(hasEitherOperand(
+ allOf(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))),
+ isTypeDependent()))),
+ // Within class templates and member functions the member expression might
+ // not be resolved. In that case, the `callExpr` is considered to be a
+ // modification.
+ callExpr(
+ callee(expr(anyOf(unresolvedMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))),
+ cxxDependentScopeMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))))))),
+ // Match on a call to a know method, but the call itself is type
+ // dependent (e.g. `vector<T> v; v.push(T{});` in a templated function).
+ callExpr(allOf(isTypeDependent(),
+ callee(memberExpr(hasDeclaration(NonConstMethod),
+ hasObjectExpression(canResolveToExpr(
+ equalsNode(Exp)))))))));
// Taking address of 'Exp'.
// We're assuming 'Exp' is mutated as soon as its address is taken, though in
@@ -234,38 +297,59 @@
unaryOperator(hasOperatorName("&"),
// A NoOp implicit cast is adding const.
unless(hasParent(implicitCastExpr(hasCastKind(CK_NoOp)))),
- hasUnaryOperand(maybeEvalCommaExpr(equalsNode(Exp))));
+ hasUnaryOperand(canResolveToExpr(equalsNode(Exp))));
const auto AsPointerFromArrayDecay =
castExpr(hasCastKind(CK_ArrayToPointerDecay),
unless(hasParent(arraySubscriptExpr())),
- has(maybeEvalCommaExpr(equalsNode(Exp))));
+ has(canResolveToExpr(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.
- const auto AsOperatorArrowThis =
- cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
- callee(cxxMethodDecl(ofClass(isMoveOnly()),
- returns(nonConstPointerType()))),
- argumentCountIs(1),
- hasArgument(0, maybeEvalCommaExpr(equalsNode(Exp))));
+ const auto AsOperatorArrowThis = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("->"),
+ callee(
+ cxxMethodDecl(ofClass(isMoveOnly()), returns(nonConstPointerType()))),
+ argumentCountIs(1), hasArgument(0, canResolveToExpr(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.
// Instantiated template functions are not handled here but in
// findFunctionArgMutation which has additional smarts for handling forwarding
// references.
- const auto NonConstRefParam = forEachArgumentWithParam(
- maybeEvalCommaExpr(equalsNode(Exp)),
- parmVarDecl(hasType(nonConstReferenceType())));
+ const auto NonConstRefParam = forEachArgumentWithParamType(
+ anyOf(canResolveToExpr(equalsNode(Exp)),
+ memberExpr(hasObjectExpression(canResolveToExpr(equalsNode(Exp))))),
+ nonConstReferenceType());
const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
+ const auto TypeDependentCallee = callee(
+ expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
+ cxxDependentScopeMemberExpr(), hasType(templateTypeParmType())
+#if 1
+ ,
+ isTypeDependent())));
+#else
+ ,
+ // Lambdas as type parameter are caught by this.
+ , ignoringParenImpCasts(declRefExpr(
+ to(varDecl(hasType(substTemplateTypeParmType()))))))));
+#endif
+
const auto AsNonConstRefArg = anyOf(
callExpr(NonConstRefParam, NotInstantiated),
cxxConstructExpr(NonConstRefParam, NotInstantiated),
- callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
- cxxDependentScopeMemberExpr(),
- hasType(templateTypeParmType())))),
- hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxUnresolvedConstructExpr(hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))));
+ callExpr(TypeDependentCallee,
+ hasAnyArgument(canResolveToExpr(equalsNode(Exp)))),
+ cxxUnresolvedConstructExpr(
+ hasAnyArgument(canResolveToExpr(equalsNode(Exp)))),
+ // Previous False Positive in the following Code:
+ // `template <typename T> void f() { int i = 42; new Type<T>(i); }`
+ // Where the constructor of `Type` takes its argument as reference.
+ // The AST does not resolve in a `cxxConstructExpr` because it is
+ // type-dependent.
+ parenListExpr(hasDescendant(expr(canResolveToExpr(equalsNode(Exp))))),
+ // If the initializer is for a reference type, there is no cast for
+ // the variable. Values are cast to RValue first.
+ initListExpr(hasAnyInit(expr(canResolveToExpr(equalsNode(Exp))))));
// Captured by a lambda by reference.
// If we're initializing a capture with 'Exp' directly then we're initializing
@@ -279,16 +363,24 @@
// For returning by const-ref there will be an ImplicitCastExpr <NoOp> (for
// adding const.)
const auto AsNonConstRefReturn = returnStmt(hasReturnValue(
- maybeEvalCommaExpr(equalsNode(Exp))));
+ anyOf(canResolveToExpr(equalsNode(Exp)),
+ castExpr(allOf(
+ hasCastKind(CK_DerivedToBase),
+ hasSourceExpression(canResolveToExpr(equalsNode(Exp))))))));
+
+ // It is used as a non-const-reference for initalizing a range-for loop.
+ const auto AsNonConstRefRangeInit = cxxForRangeStmt(
+ hasRangeInit(declRefExpr(allOf(canResolveToExpr(equalsNode(Exp)),
+ hasType(nonConstReferenceType())))));
const auto Matches = match(
- traverse(
- ast_type_traits::TK_AsIs,
- findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, AsNonConstThis,
- AsAmpersandOperand, AsPointerFromArrayDecay,
- AsOperatorArrowThis, AsNonConstRefArg,
- AsLambdaRefCaptureInit, AsNonConstRefReturn))
- .bind("stmt"))),
+ traverse(ast_type_traits::TK_AsIs,
+ findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand,
+ AsNonConstThis, AsAmpersandOperand,
+ AsPointerFromArrayDecay, AsOperatorArrowThis,
+ AsNonConstRefArg, AsLambdaRefCaptureInit,
+ AsNonConstRefReturn, AsNonConstRefRangeInit))
+ .bind("stmt"))),
Stm, Context);
return selectFirst<Stmt>("stmt", Matches);
}
@@ -296,9 +388,10 @@
const Stmt *ExprMutationAnalyzer::findMemberMutation(const Expr *Exp) {
// Check whether any member of 'Exp' is mutated.
const auto MemberExprs =
- match(findAll(expr(anyOf(memberExpr(hasObjectExpression(equalsNode(Exp))),
- cxxDependentScopeMemberExpr(
- hasObjectExpression(equalsNode(Exp)))))
+ match(findAll(expr(anyOf(memberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))),
+ cxxDependentScopeMemberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp))))))
.bind(NodeID<Expr>::value)),
Stm, Context);
return findExprMutation(MemberExprs);
@@ -306,43 +399,112 @@
const Stmt *ExprMutationAnalyzer::findArrayElementMutation(const Expr *Exp) {
// Check whether any element of an array is mutated.
- const auto SubscriptExprs = match(
- findAll(arraySubscriptExpr(hasBase(ignoringImpCasts(equalsNode(Exp))))
- .bind(NodeID<Expr>::value)),
- Stm, Context);
+ const auto SubscriptExprs =
+ match(findAll(arraySubscriptExpr(
+ anyOf(hasBase(canResolveToExpr(equalsNode(Exp))),
+ hasBase(implicitCastExpr(
+ allOf(hasCastKind(CK_ArrayToPointerDecay),
+ hasSourceExpression(canResolveToExpr(
+ equalsNode(Exp))))))))
+ .bind(NodeID<Expr>::value)),
+ Stm, Context);
return findExprMutation(SubscriptExprs);
}
const Stmt *ExprMutationAnalyzer::findCastMutation(const Expr *Exp) {
+ // If the 'Exp' is explicitly casted to a non-const reference type the
+ // 'Exp' is considered to be modified.
+ const auto ExplicitCast = match(
+ findAll(
+ stmt(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))),
+ explicitCastExpr(
+ hasDestinationType(nonConstReferenceType()))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *CastStmt = selectFirst<Stmt>("stmt", ExplicitCast))
+ return CastStmt;
+
// If 'Exp' is casted to any non-const reference type, check the castExpr.
- const auto Casts =
- match(findAll(castExpr(hasSourceExpression(equalsNode(Exp)),
- anyOf(explicitCastExpr(hasDestinationType(
- nonConstReferenceType())),
- implicitCastExpr(hasImplicitDestinationType(
- nonConstReferenceType()))))
- .bind(NodeID<Expr>::value)),
- Stm, Context);
+ const auto Casts = match(
+ findAll(
+ expr(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))),
+ anyOf(explicitCastExpr(
+ hasDestinationType(nonConstReferenceType())),
+ implicitCastExpr(hasImplicitDestinationType(
+ nonConstReferenceType())))))
+ .bind(NodeID<Expr>::value)),
+ Stm, Context);
+
if (const Stmt *S = findExprMutation(Casts))
return S;
// Treat std::{move,forward} as cast.
const auto Calls =
match(findAll(callExpr(callee(namedDecl(
hasAnyName("::std::move", "::std::forward"))),
- hasArgument(0, equalsNode(Exp)))
+ hasArgument(0, canResolveToExpr(equalsNode(Exp))))
.bind("expr")),
Stm, Context);
return findExprMutation(Calls);
}
const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) {
+ // Keep the ordering for the specific initialization matches to happen first,
+ // because it is cheaper to match then all potential modifications of the
+ // loop variable.
+
+ // The range variable is a reference to a builtin array. In that case the
+ // array is considered modified if the loop-variable is a non-const reference.
+ const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
+ hasUnqualifiedDesugaredType(referenceType(pointee(arrayType())))))));
+ const auto RefToArrayRefToElements = match(
+ findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
+ .bind(NodeID<Decl>::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *BadRangeInitFromArray =
+ selectFirst<Stmt>("stmt", RefToArrayRefToElements))
+ return BadRangeInitFromArray;
+
+ // Small helper to match special cases in range-for loops.
+ //
+ // It is possible that containers do not provide a const-overload for their
+ // iterator accessors. If this is the case, the variable is used non-const
+ // no matter what happens in the loop. This requires special detection as it
+ // is then faster to find all mutations of the loop variable.
+ // It aims at a different modification as well.
+ const auto HasAnyNonConstIterator =
+ anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst()))),
+ unless(hasMethod(allOf(hasName("begin"), isConst())))),
+ allOf(hasMethod(allOf(hasName("end"), unless(isConst()))),
+ unless(hasMethod(allOf(hasName("end"), isConst())))));
+
+ const auto DeclStmtToNonConstIteratorContainer = declStmt(
+ hasSingleDecl(varDecl(hasType(hasUnqualifiedDesugaredType(referenceType(
+ pointee(hasDeclaration(cxxRecordDecl(HasAnyNonConstIterator)))))))));
+
+ const auto RefToContainerBadIterators =
+ match(findAll(stmt(cxxForRangeStmt(allOf(
+ hasRangeStmt(DeclStmtToNonConstIteratorContainer),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp))))))
+ .bind("stmt")),
+ Stm, Context);
+
+ if (const auto *BadIteratorsContainer =
+ selectFirst<Stmt>("stmt", RefToContainerBadIterators))
+ return BadIteratorsContainer;
+
// If range for looping over 'Exp' with a non-const reference loop variable,
// check all declRefExpr of the loop variable.
const auto LoopVars =
match(findAll(cxxForRangeStmt(
hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
.bind(NodeID<Decl>::value)),
- hasRangeInit(equalsNode(Exp)))),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp))))),
Stm, Context);
return findDeclMutation(LoopVars);
}
@@ -356,7 +518,8 @@
hasOverloadedOperatorName("*"),
callee(cxxMethodDecl(ofClass(isMoveOnly()),
returns(nonConstReferenceType()))),
- argumentCountIs(1), hasArgument(0, equalsNode(Exp)))
+ argumentCountIs(1),
+ hasArgument(0, canResolveToExpr(equalsNode(Exp))))
.bind(NodeID<Expr>::value)),
Stm, Context);
if (const Stmt *S = findExprMutation(Ref))
@@ -367,13 +530,12 @@
stmt(forEachDescendant(
varDecl(
hasType(nonConstReferenceType()),
- hasInitializer(anyOf(equalsNode(Exp),
- conditionalOperator(anyOf(
- hasTrueExpression(equalsNode(Exp)),
- hasFalseExpression(equalsNode(Exp)))))),
+ hasInitializer(anyOf(canResolveToExpr(equalsNode(Exp)),
+ memberExpr(hasObjectExpression(
+ canResolveToExpr(equalsNode(Exp)))))),
hasParent(declStmt().bind("stmt")),
- // Don't follow the reference in range statement, we've handled
- // that separately.
+ // Don't follow the reference in range statement, we've
+ // handled that separately.
unless(hasParent(declStmt(hasParent(
cxxForRangeStmt(hasRangeStmt(equalsBoundNode("stmt"))))))))
.bind(NodeID<Decl>::value))),
@@ -383,7 +545,7 @@
const Stmt *ExprMutationAnalyzer::findFunctionArgMutation(const Expr *Exp) {
const auto NonConstRefParam = forEachArgumentWithParam(
- equalsNode(Exp),
+ canResolveToExpr(equalsNode(Exp)),
parmVarDecl(hasType(nonConstReferenceType())).bind("parm"));
const auto IsInstantiated = hasDeclaration(isInstantiated());
const auto FuncDecl = hasDeclaration(functionDecl().bind("func"));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
