[PATCH] D52219: [analyzer] (1/n) Support pointee mutation analysis in ExprMutationAnalyzer.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang marked 10 inline comments as done.
shuaiwang added inline comments.


================
Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:86
+// - Pointer to non-const
+// - Pointer-like type with `operator*` returning non-const reference
+bool isPointeeMutable(const Expr *Exp, const ASTContext &Context) {
----------------
Szelethus wrote:
> Didn't you mean these to be doxygen comments?
I feel these don't need to be doxygen comments.


================
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
+
+  AST = tooling::buildASTFromCode(
----------------
JonasToth wrote:
> JonasToth wrote:
> > I feel that there a multiple tests missing:
> > 
> > - multiple levels of pointers `int ***`, `int * const *`
> > - pointers to references `int &*`
> > - references to pointers `int *&`
> > - ensure that having a const pointer does no influence the pointee analysis `int * const p = &i; *p = 42;`
> > - a class with `operator*` + `operator->` const/non-const and the analysis for pointers to that class
> > - pointer returned from a function
> > - non-const reference returned 
> > ```
> > int& foo(int *p) {
> >   return *p;
> > }
> > ```
> > 
> for the multi-level pointer mutation: it would be enough to test, that the second layer is analyzed properly, and that the `int * >const< *` would be detected.
Added except for:
- Anything that requires following a dereference, we need `findPointeeDerefMutation` for that.
- Pointer to a class with `operator*` + `operator->`, I think those two operators doesn't matter, there's no way to accidentally invoke them from a pointer.



================
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:686
+  AST = tooling::buildASTFromCode(
+      "void g(const int*); void f() { const int x[2] = {}; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
----------------
JonasToth wrote:
> Please add a mutating example for array access through a pointer (as its very common in C-style code).
This also need `findPointeeDerefMutation`.


Repository:
  rC Clang

https://reviews.llvm.org/D52219