shuaiwang added inline comments.
================ Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:82-83 + const auto *E = RefNodes.getNodeAs<Expr>("expr"); + if (findMutation(E)) + return E; + } ---------------- aaron.ballman wrote: > Why does this not return the result of `findMutation()` like the other call > above? find*Mutation is intended to return where the given Expr is mutated. To make it a bit more useful, for Decl's I'm returning the DeclRefExpr and caller can chose to follow it to find where the DeclRefExpr is mutated if needed. As a concrete example: ``` struct A { int x; }; struct B { A a; }; struct C { B b; }; C c; C& c2 = c; c2.b.a.x = 10; ``` If we start with DeclRefExpr to `c` we'll find mutation at a DeclRefExpr to `c2`, following that we can find the mutation Stmt being `c2.b.a.x = 10`. Returning `E` here makes it possible to reveal intermediate checkpoint instead of directly saying `c` is mutated at `c2.b.a.x = 10` which might be confusing. ================ Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88 + +const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { + // LHS of any assignment operators. ---------------- aaron.ballman wrote: > Should this also consider a DeclRefExpr to a volatile-qualified variable as a > direct mutation? > > What about using `Expr::HasSideEffect()`? Good catch about DeclRefExpr to volatile. `HasSideEffects` means something different. Here find*Mutation means find a Stmt **in ancestors** that mutates the given Expr. `HasSideEffects` IIUC means whether anything is mutated by the Expr but doesn't care about what exactly is mutated. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45679 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits