diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp --- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp +++ b/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, InnerMatcher) { - const Expr* Result = &Node; +AST_MATCHER_P(Expr, maybeEvalCommaExpr, ast_matchers::internal::Matcher, + InnerMatcher) { + const Expr *Result = &Node; while (const auto *BOComma = - dyn_cast_or_null(Result->IgnoreParens())) { + dyn_cast_or_null(Result->IgnoreParens())) { if (!BOComma->isCommaOp()) break; Result = BOComma->getRHS(); @@ -36,6 +39,55 @@ return InnerMatcher.matches(*Result, Finder, Builder); } +AST_MATCHER_P(Expr, canResolveToExpr, ast_matchers::internal::Matcher, + InnerMatcher) { + auto DerivedToBase = [](const ast_matchers::internal::Matcher &Inner) { + return implicitCastExpr(anyOf(hasCastKind(CK_DerivedToBase), + hasCastKind(CK_UncheckedDerivedToBase)), + hasSourceExpression(Inner)); + }; + auto IgnoreDerivedToBase = + [&DerivedToBase](const ast_matchers::internal::Matcher &Inner) { + return ignoringParens(expr(anyOf(Inner, DerivedToBase(Inner)))); + }; + + // The 'ConditionalOperator' matches on ` ? : `. + // This matching must be recursive because `` can be anything resolving + // to the `InnerMatcher`, for example another conditional operator. + // The edge-case `BaseClass &b = ? DerivedVar1 : DerivedVar2;` + // is handled, too. The implicit cast happens outside of the conditional. + // This is matched by `IgnoreDerivedToBase(canResolveToExpr(InnerMatcher))` + // below. + auto const ConditionalOperator = conditionalOperator(anyOf( + hasTrueExpression(ignoringParens(canResolveToExpr(InnerMatcher))), + hasFalseExpression(ignoringParens(canResolveToExpr(InnerMatcher))))); + auto const ElvisOperator = binaryConditionalOperator(anyOf( + hasTrueExpression(ignoringParens(canResolveToExpr(InnerMatcher))), + hasFalseExpression(ignoringParens(canResolveToExpr(InnerMatcher))))); + + auto const ComplexMatcher = ignoringParens( + expr(anyOf(IgnoreDerivedToBase(InnerMatcher), + maybeEvalCommaExpr(IgnoreDerivedToBase(InnerMatcher)), + IgnoreDerivedToBase(ConditionalOperator), + IgnoreDerivedToBase(ElvisOperator)))); + + 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, + 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 cxxTypeidExpr; @@ -151,7 +203,7 @@ NodeID::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 +254,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 known method, but the call itself is type + // dependent (e.g. `vector 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 +300,51 @@ 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()), isTypeDependent()))); + 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 void f() { int i = 42; new Type(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 @@ -278,17 +357,22 @@ // For returning by value there will be an ImplicitCastExpr . // For returning by const-ref there will be an ImplicitCastExpr (for // adding const.) - const auto AsNonConstRefReturn = returnStmt(hasReturnValue( - maybeEvalCommaExpr(equalsNode(Exp)))); + const auto AsNonConstRefReturn = + returnStmt(hasReturnValue(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", Matches); } @@ -296,9 +380,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::value)), Stm, Context); return findExprMutation(MemberExprs); @@ -306,43 +391,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::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::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", 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::value)), - Stm, Context); + const auto Casts = match( + findAll( + expr(castExpr(hasSourceExpression(canResolveToExpr(equalsNode(Exp))), + anyOf(explicitCastExpr( + hasDestinationType(nonConstReferenceType())), + implicitCastExpr(hasImplicitDestinationType( + nonConstReferenceType()))))) + .bind(NodeID::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 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::value)), + hasRangeStmt(DeclStmtToNonRefToArray), + hasRangeInit(canResolveToExpr(equalsNode(Exp))))) + .bind("stmt")), + Stm, Context); + + if (const auto *BadRangeInitFromArray = + selectFirst("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", 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::value)), - hasRangeInit(equalsNode(Exp)))), + hasRangeInit(canResolveToExpr(equalsNode(Exp))))), Stm, Context); return findDeclMutation(LoopVars); } @@ -356,7 +510,8 @@ hasOverloadedOperatorName("*"), callee(cxxMethodDecl(ofClass(isMoveOnly()), returns(nonConstReferenceType()))), - argumentCountIs(1), hasArgument(0, equalsNode(Exp))) + argumentCountIs(1), + hasArgument(0, canResolveToExpr(equalsNode(Exp)))) .bind(NodeID::value)), Stm, Context); if (const Stmt *S = findExprMutation(Ref)) @@ -367,13 +522,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::value))), @@ -383,7 +537,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")); diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp --- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp +++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include "clang/AST/TypeLoc.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/Tooling.h" @@ -19,9 +20,7 @@ using namespace clang::ast_matchers; using ::testing::ElementsAre; -using ::testing::IsEmpty; using ::testing::ResultOf; -using ::testing::StartsWith; using ::testing::Values; namespace { @@ -63,12 +62,16 @@ const auto *const S = selectFirst("stmt", Results); SmallVector Chain; ExprMutationAnalyzer Analyzer(*S, AST->getASTContext()); + for (const auto *E = selectFirst("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(By); } return Chain; @@ -111,7 +114,13 @@ class AssignmentTest : public ::testing::TestWithParam {}; +// This test is for the most basic and direct modification of a variable, +// assignment to it (e.g. `x = 10;`). +// It additionally tests that references to a variable are not only captured +// directly but expressions 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 +129,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 +137,89 @@ 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 the 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)); + } + + // Test the detection for the binary conditional operator. + { + const std::string ModExpr = "(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 {}; TEST_P(IncDecTest, IncDecModifies) { @@ -147,6 +234,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 +274,18 @@ EXPECT_FALSE(isMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, TypeDependentMemberCall) { + const auto AST = buildASTFromCodeWithArgs( + "template class vector { void push_back(T); }; " + "template void f() { vector 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 +302,19 @@ EXPECT_FALSE(isMutated(Results, AST.get())); } +TEST(ExprMutationAnalyzerTest, UnresolvedOperator) { + const auto AST = buildASTFromCodeWithArgs( + "template 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 +436,22 @@ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("A<0>::mf(x)")); } +TEST(ExprMutationAnalyzerTest, ByNonConstRefArgumentFunctionTypeDependent) { + auto AST = buildASTFromCodeWithArgs( + "enum MyEnum { foo, bar };" + "void tryParser(unsigned& first, MyEnum Type) { first++, (void)Type; }" + "template void parse() {" + " auto parser = [](unsigned& first) { first++; tryParser(first, Type); " + "};" + " unsigned x = 42;" + " parser(x);" + "}", + {"-fno-delayed-template-parsing"}); + auto Results = + match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); + EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("parser(x)")); +} + TEST(ExprMutationAnalyzerTest, ByConstRefArgument) { auto AST = buildASTFromCode("void g(const int&); void f() { int x; g(x); }"); auto Results = @@ -394,24 +524,30 @@ "void g(const int&&); void f() { int x; g(static_cast(x)); }"); auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);" "void f() { A x; static_cast(x) + 1; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("void f() { struct A { A(const int&&); }; " "int x; A y(static_cast(x)); }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(x)")); AST = buildASTFromCode("void f() { struct A { A(); A(const A&&); }; " "A x; A y(static_cast(x)); }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); - EXPECT_FALSE(isMutated(Results, AST.get())); + EXPECT_THAT(mutatedBy(Results, AST.get()), + ElementsAre("static_cast(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 +626,9 @@ ElementsAre("std::forward(x) = y")); } +// section: template constellations that prohibit reasoning about modifications +// as it depends on instantiations. + TEST(ExprMutationAnalyzerTest, CallUnresolved) { auto AST = buildASTFromCodeWithArgs("template void f() { T x; g(x); }", @@ -543,6 +682,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 +720,7 @@ const auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("return static_cast(x);")); + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, ReturnAsConstRRef) { @@ -587,9 +728,12 @@ "const int&& f() { int x; return static_cast(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(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 +765,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 +939,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 +955,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 +1000,8 @@ EXPECT_FALSE(isMutated(Results, AST.get())); } +// section: casts + TEST(ExprMutationAnalyzerTest, CastToValue) { const auto AST = buildASTFromCode("void f() { int x; static_cast(x); }"); @@ -863,13 +1016,13 @@ auto Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("static_cast(x) = 10")); + ElementsAre("static_cast(x)")); AST = buildASTFromCode("typedef int& IntRef;" "void f() { int x; static_cast(x) = 10; }"); Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext()); EXPECT_THAT(mutatedBy(Results, AST.get()), - ElementsAre("static_cast(x) = 10")); + ElementsAre("static_cast(x)")); } TEST(ExprMutationAnalyzerTest, CastToRefNotModified) { @@ -877,7 +1030,8 @@ buildASTFromCode("void f() { int x; static_cast(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(x)")); } TEST(ExprMutationAnalyzerTest, CastToConstRef) { @@ -893,6 +1047,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 +1175,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 +1217,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 +1279,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 +1288,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 +1309,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 +1364,8 @@ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()")); } +// section: special case: smartpointers + TEST(ExprMutationAnalyzerTest, UniquePtr) { const std::string UniquePtrDef = "template struct UniquePtr {" @@ -1246,6 +1423,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 " + "struct to_construct : T { to_construct(int &j) {} };" + "template " + "void placement_new_in_unique_ptr() { int x = 0;" + " new to_construct(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 {"