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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
