JonasToth updated this revision to Diff 294300.
JonasToth added a comment.
- address review comments
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,6 +136,60 @@
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,
@@ -147,6 +210,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 +250,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()); }",
+ {"-Wno-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 +278,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 =
@@ -394,22 +484,26 @@
"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)"));
}
TEST(ExprMutationAnalyzerTest, Move) {
@@ -579,7 +673,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,7 +681,8 @@
"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)"));
}
TEST(ExprMutationAnalyzerTest, TakeAddress) {
@@ -863,13 +958,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 +972,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) {
@@ -1054,20 +1150,22 @@
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 +1205,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 +1214,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) {
@@ -1246,6 +1345,22 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x->mf()"));
}
+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,40 @@
return InnerMatcher.matches(*Result, Finder, Builder);
}
+AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher<Expr>,
+ InnerMatcher) {
+ // Matches unless the value is a derived class and is assigned to a
+ // reference to the base class. Other implicit casts should not happen.
+ const auto IgnoreDerivedToBase = ignoringParens(
+ expr(anyOf(implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase),
+ hasCastKind(CK_UncheckedDerivedToBase)),
+ hasSourceExpression(InnerMatcher)),
+ InnerMatcher)));
+
+ auto const ComplexMatcher = ignoringParens(
+ expr(anyOf(maybeEvalCommaExpr(IgnoreDerivedToBase),
+ conditionalOperator(
+ anyOf(hasTrueExpression(ignoringParens(
+ canResolveToExpr(IgnoreDerivedToBase))),
+ hasFalseExpression(ignoringParens(
+ canResolveToExpr(IgnoreDerivedToBase))))))));
+ 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 +188,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 +239,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,29 +285,29 @@
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 AsNonConstRefArg = anyOf(
callExpr(NonConstRefParam, NotInstantiated),
@@ -264,8 +315,18 @@
callExpr(callee(expr(anyOf(unresolvedLookupExpr(), unresolvedMemberExpr(),
cxxDependentScopeMemberExpr(),
hasType(templateTypeParmType())))),
- hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))),
- cxxUnresolvedConstructExpr(hasAnyArgument(maybeEvalCommaExpr(equalsNode(Exp)))));
+ 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 +340,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 +365,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 +376,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 +495,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 +507,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 +522,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
