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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

TDD, thats ok ;)

Am 22.09.2018 um 19:37 schrieb Shuai Wang via Phabricator:

> shuaiwang added inline comments.
> 
> ================
>  Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:156
> 
>   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.mf()"));
> 
> +
>  +  AST = tooling::buildASTFromCode(
> 
>  ----------------
> 
> JonasToth wrote:
> 
>> shuaiwang wrote:
>> 
>>> 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.
>> 
>> But we want to analyze smart pointers in the future as well, not? It would be good to already prepare that in the testing department.
>>  Or would the nature of `operator*` already make that happen magically?
> 
> Yes we'll handle smart pointers, and we'll handle that in `findPointeeDerefMutation`, basically it'll look like:
> 
>   if (native pointer && derefed with *) findMutation(deref expr)
>   if (smart pointer && called operator*) findMutation(operator call expr)
>   if (smart pointer && called operator->) findPointeeMutation(operator call expr)
> 
> 
> I think it would be more clear if we can match the implementation progress with unit test cases as that shows what kind of cases starts to be supported by the change.
> 
> Repository:
> 
>   rC Clang
> 
> https://reviews.llvm.org/D52219


Repository:
  rC Clang

https://reviews.llvm.org/D52219