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

Reply via email to