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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
