[PATCH] D54563: [analyzer] MoveChecker Pt.4: Add a few more state reset methods.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 3 17:03:56 PST 2018


NoQ added a comment.

I'm glad you brought this stuff up, found two more bugs to fix.

In D54563#1317678 <https://reviews.llvm.org/D54563#1317678>, @Szelethus wrote:

> Can you add tests for that just in case? :)


The test works, but i noticed that we aren't respecting const references. That is, we believe that passing the object by const reference or pointer may reset it. I guess i'll make a separate commit.



================
Comment at: test/Analysis/use-after-move.cpp:260-262
     for (int i = 0; i < bignum(); i++) { // expected-note {{Loop condition is false. Execution jumps to the end of the function}}
       rightRefCall(std::move(a));        // no-warning
     }
----------------
Szelethus wrote:
> NoQ wrote:
> > This would have been the test for our case, but in this test the function has a body and will not be evaluated conservatively.
> Hmm, since those are all STL containers, we should be able to have their definition? Or only with `c++-container-inlining=true`? Take this as more of a question than anything else.
Yeah, assuming that we only need containers.

As usual, i expect only good things to happen when we inline them. Say, if a field is overwritten in the inlined call, this would also trigger `checkRegionChanges`. And i doubt that in these examples there will be a path through the function that leaves the object entirely untouched.

As a side note, i doubt that we'll actually react to field assignment, because this checker's `checkRegionChanges` callback doesn't account for that: it only iterates through explicit regions. I guess this needs fixing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54563/new/

https://reviews.llvm.org/D54563





More information about the cfe-commits mailing list