[clang] improve accuracy of ExprMutAnalyzer

[JonasToth]

Summary

This patch extracts the ExprMutAnalyzer changes from https://reviews.llvm.org/D54943
into its own revision for simpler review and more atomic changes.

[aaron.ballman]

clang/lib/Analysis/ExprMutationAnalyzer.cpp

44

Unless the value -> Matches unless the value

65

I think I'd prefer this to be named hasAnyInit() to complement hasInit() -- these aren't really arguments.

275

operator call expression -> The call operator expression

277

moditication -> modification

345

casted -> cast

465

It is possible, that -> It is possible that

468

is faster to find then all -> is then faster to find all

471

Do we want to look for methods that end with _?[Bb]egin or _?[Ee]nd so that this would catch patterns like foo_begin()/foo_end(), FooBegin()/FooEnd(), or Foo_Begin()/Foo_End()?

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

66

Was there a reason you hoisted this out of the for loop?

[JonasToth]

• improve the expression matcher, to catch the expression in general ternary-operator cases, too
• considered unresolved operator calls to be a modification
• fix method calls with templates in some instances

[JonasToth]

• adress review comments

clang/lib/Analysis/ExprMutationAnalyzer.cpp

471

This specific matcher is only applied in range-for contexts. There only the begin(); end() methods matter. I updated the comment above to clarify this.

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

66

Jup.

buffer.clear()

The current form does proper memory-recycling (i believe at least :D)

[aaron.ballman]

LGTM aside from a nit.

clang/lib/Analysis/ExprMutationAnalyzer.cpp

471

Ah, okay, that makes a lot more sense, thanks!

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

66

This smells like a micro-optimization to me (I think declaring the variable in the loop is more clear than clearing the buffer on each iteration).

I don't insist on changing it, but if you want to keep it, you should name it Buffer per our usual coding rules.

[JonasToth]

• address review comments

[JonasToth]

• fix typo that provided wrong argument to AST building

[JonasToth]

• 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

[JonasToth]

@aaron.ballman I update the code a bit. This stuff is just a hydra :/

But i think incrementally the current version is better, but still not perfect. I update the clang-tidy part, too.

Edit: from scrolling through there are still rough edges in the current version. i will remove the stuff tomorrow, right now i am to tired :/

[JonasToth]

• fix imprecisions

[aaron.ballman]

LGTM though I may have spotted a potential improvement (it can be handled in a follow-up if you want).

clang/lib/Analysis/ExprMutationAnalyzer.cpp

61

Do you also want to handle the binary conditional operator (a GNU extension like a ? : b)?

[JonasToth]

• [Feature] add elvis operator detection

[JonasToth]

clang/lib/Analysis/ExprMutationAnalyzer.cpp

61

Yup, was a fast improvement. Thanks for pointing that out 👍

clang/lib/Analysis/ExprMutationAnalyzer.cpp

 //===---------- ExprMutationAnalyzer.cpp ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #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 {
 using namespace ast_matchers;
 
 namespace {
 
 AST_MATCHER_P(LambdaExpr, hasCaptureInit, const Expr *, E) {
   return llvm::is_contained(Node.capture_inits(), E);
 }
 
 AST_MATCHER_P(CXXForRangeStmt, hasRangeStmt,
               ast_matchers::internal::Matcher<DeclStmt>, InnerMatcher) {
   const DeclStmt *const Range = Node.getRangeStmt();
   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())) {
     if (!BOComma->isCommaOp())
       break;
     Result = BOComma->getRHS();
   }
   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));
+  };
+  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 `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<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;
 
 AST_MATCHER(CXXTypeidExpr, isPotentiallyEvaluated) {
   return Node.isPotentiallyEvaluated();
 }
 
 const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt,
const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl *Dec,
   return nullptr;
 }
 
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
   return selectFirst<Expr>(
              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
                               // typeLoc.
                               hasAncestor(typeLoc(unless(
                                   hasAncestor(unaryExprOrTypeTraitExpr())))),
                               hasAncestor(expr(anyOf(
                                   // `UnaryExprOrTypeTraitExpr` is unevaluated
const Stmt *ExprMutationAnalyzer::findDeclPointeeMutation(
     ArrayRef<ast_matchers::BoundNodes> Matches) {
   return tryEachMatch<Decl>(Matches, this,
                             &ExprMutationAnalyzer::findPointeeMutation);
 }
 
 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)))),
+                          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<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
   // theory we can follow the pointer and see whether it escaped `Stm` or is
   // dereferenced and then mutated. This is left for future improvements.
   const auto AsAmpersandOperand =
       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 <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
   // a reference capture.
   // For value captures there will be an ImplicitCastExpr <LValueToRValue>.
   const auto AsLambdaRefCaptureInit = lambdaExpr(hasCaptureInit(Exp));
 
   // Returned as non-const-ref.
   // If we're returning 'Exp' directly then it's returned as non-const-ref.
   // For returning by value there will be an ImplicitCastExpr <LValueToRValue>.
   // For returning by const-ref there will be an ImplicitCastExpr <NoOp> (for
   // adding const.)
-  const auto AsNonConstRefReturn = returnStmt(hasReturnValue(
-                                                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))
+      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);
 }
 
 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);
 }
 
 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))))
+  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())),
+                                  nonConstReferenceType())))))
+
   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<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);
 }
 
 const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) {
   // Follow non-const reference returned by `operator*()` of move-only classes.
   // These are typically smart pointers with unique ownership so we treat
   // mutation of pointee as mutation of the smart pointer itself.
   const auto Ref =
       match(findAll(cxxOperatorCallExpr(
                         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))
     return S;
 
   // If 'Exp' is bound to a non-const reference, check all declRefExpr to that.
   const auto Refs = match(
       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))),
       Stm, Context);
   return findDeclMutation(Refs);
 }
 
 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"));
   const auto Matches = match(
       traverse(
           ast_type_traits::TK_AsIs,
           findAll(
               expr(anyOf(callExpr(NonConstRefParam, IsInstantiated, FuncDecl,

clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

 //===---------- ExprMutationAnalyzerTest.cpp ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "llvm/ADT/SmallString.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cctype>
 
 namespace clang {
 
 using namespace clang::ast_matchers;
 using ::testing::ElementsAre;
-using ::testing::IsEmpty;
 using ::testing::ResultOf;
-using ::testing::StartsWith;
 using ::testing::Values;
 
 namespace {
 
 using ExprMatcher = internal::Matcher<Expr>;
 using StmtMatcher = internal::Matcher<Stmt>;
 
 std::unique_ptr<ASTUnit>
bool isMutated(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
   return ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
 }
 
 SmallVector<std::string, 1>
 mutatedBy(const SmallVectorImpl<BoundNodes> &Results, ASTUnit *AST) {
   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;
 }
 
 std::string removeSpace(std::string s) {
   s.erase(std::remove_if(s.begin(), s.end(),
                          [](char c) { return llvm::isSpace(c); }),
TEST(ExprMutationAnalyzerTest, Trivial) {
   const auto AST = buildASTFromCode("void f() { int x; x; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 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 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 + "; }");
     const auto Results =
         match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
     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 + "; }");
     const auto Results =
         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<std::string> {};
 
 TEST_P(IncDecTest, IncDecModifies) {
   const std::string ModExpr = GetParam();
   const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
 }
 
 INSTANTIATE_TEST_CASE_P(AllIncDecOperators, IncDecTest,
                         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(); }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
 }
 

 TEST(ExprMutationAnalyzerTest, ConstMemberFunc) {
   const auto AST = buildASTFromCode(
       "void f() { struct Foo { void mf() const; }; Foo x; x.mf(); }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, ConstOperator) {
   const auto AST = buildASTFromCode(
       "void f() { struct Foo { int operator()() const; }; Foo x; x(); }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode("void g(int*); void f() { int* x; g(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
TEST(ExprMutationAnalyzerTest, ByNonConstRefArgument) {
 
   AST = buildASTFromCode(
       "template <int> struct A { A(); A(const A&); static void mf(A&) {} };"
       "void f() { A<0> x; A<0>::mf(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 <MyEnum Type> 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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode("typedef const int& CIntRef;"
                          "void g(CIntRef); void f() { int x; g(x); }");
EXPECT_THAT(mutatedBy(Results, AST.get()),
               ElementsAre("static_cast<A &&>(x)"));
 }
 
 TEST(ExprMutationAnalyzerTest, ByConstRRefArgument) {
   auto AST = buildASTFromCode(
       "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); }");
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(StdRemoveReference + StdMove +
TEST(ExprMutationAnalyzerTest, Forward) {
   AST = buildASTFromCode(
       StdRemoveReference + StdForward +
       "void f() { struct A {}; A x, y; std::forward<A &>(x) = y; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
               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); }",
                                {"-fno-delayed-template-parsing"});
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
 
TEST(ExprMutationAnalyzerTest, CallUnresolved) {
 
   AST = buildASTFromCodeWithArgs(
       "template <class T> void f() { int x; (void)T(x); }",
       {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode("int* f() { int* x; return x; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());

 }
 
 TEST(ExprMutationAnalyzerTest, ReturnAsNonConstRRef) {
   const auto AST =
       buildASTFromCode("int&& f() { int x; return static_cast<int &&>(x); }");
   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) {
   const auto AST = buildASTFromCode(
       "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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("&x"));
 }
 
 TEST(ExprMutationAnalyzerTest, ArrayToPointerDecay) {
TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
   const auto ResultsX =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(ResultsX, AST.get()), ElementsAre("g(x)"));
   const auto ResultsY =
       match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
   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; "
       "int& r3 = r2; r3 = 10; }");
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
               ElementsAre("r0", "r1", "r2", "r3", "r3 = 10"));
AST = buildASTFromCode(StdRemoveReference + StdForward +
                          "{ u(std::forward<Args>(args)...); }"
                          "template <class... Args> void g(Args&&... args)"
                          "{ h(std::forward<Args>(args)...); }"
                          "void f() { int x; g(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x[0] = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, ArrayElementNotModified) {
   const auto AST = buildASTFromCode("void f() { int x[2]; x[0]; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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; }; "
                        "struct C { B vb; }; C x; x.vb.va.vi = 10; }");
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.vb.va.vi = 10"));
 
TEST(ExprMutationAnalyzerTest, NestedMemberNotModified) {
   AST =
       buildASTFromCodeWithArgs("template <class T> struct S;"
                                "template <class T> void f() { S<T> x; x.y.z; }",
                                {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
+// section: casts
+
 TEST(ExprMutationAnalyzerTest, CastToValue) {
   const auto AST =
       buildASTFromCode("void f() { int x; static_cast<double>(x); }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, CastToRefModified) {
   auto AST =
       buildASTFromCode("void f() { int x; static_cast<int &>(x) = 10; }");
   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) {
   const auto AST =
       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) {
   auto AST =
       buildASTFromCode("void f() { int x; static_cast<const int&>(x); }");
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode("typedef const int& CIntRef;"
                          "void f() { int x; static_cast<CIntRef>(x); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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"});
   const auto Results =
       match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
const auto AST = buildASTFromCodeWithArgs(
                      "{ UniquePtr<T> x; UniquePtr<T> y;"
                      " (y, x)->mf(); }",
       {"-fno-delayed-template-parsing", "-Wno-unused-value"});
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
+TEST(ExprMutationAnalyzerTest, 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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByValue) {

 TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByRef) {
   const auto AST = buildASTFromCode("void f() { int x; [&x]() { x = 10; }; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
               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) {
   auto AST = buildASTFromCode("void f() { int x[2]; for (int e : x) e = 10; }");
   auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 

 }
 
 TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefModified) {
   const auto AST =
       buildASTFromCode("struct V { int* begin(); int* end(); };"
                        "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) {
   const auto AST = buildASTFromCode("struct V { int* begin(); int* end(); };"
                                     "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) {
   const auto AST = buildASTFromCode(
       "struct V { const int* begin() const; const int* end() const; };"
       "void f() { V x; for (int e : x) e; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForNonArrayByConstRef) {
   const auto AST = buildASTFromCode(
       "struct V { const int* begin() const; const int* end() const; };"
       "void f() { V x; for (const int& e : x) e; }");
   const auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode("void f() { int x, y; __typeof(x = 10) z = y; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
AST = buildASTFromCodeWithArgs(
       "namespace std { class type_info; }"
       "struct A { virtual ~A(); }; struct B : A {};"
       "struct X { A& f(); }; void f() { X x; typeid(x.f()); }",
       {"-frtti"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 {"
       "  UniquePtr();"
       "  UniquePtr(const UniquePtr&) = delete;"
       "  UniquePtr(UniquePtr&&);"
       "  UniquePtr& operator=(const UniquePtr&) = delete;"
       "  UniquePtr& operator=(UniquePtr&&);"
TEST(ExprMutationAnalyzerTest, UniquePtr) {
 
   AST = buildASTFromCodeWithArgs(
       UniquePtrDef + "template <class T> void f() { UniquePtr<T> x; x->mf(); }",
       {"-fno-delayed-template-parsing"});
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   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 {"
       "template <class T> T forward(T & A) { return static_cast<T&&>(A); }"
       "template <class T> struct __bind {"
       "  T f;"
       "  template <class V> __bind(T v, V &&) : f(forward(v)) {}"
       "};"