[PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

[Artem Dergachev via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D22374#505672, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D22374#504855, @NoQ wrote:
>
> > Hmm. I suggest:
> >
> > 1. Change this test's constructor so that it was no longer almost-trivial. Because it isn't significant for this test if the constructor is almost-trivial or not. The test would pass.
>
>
> It is significant, because I want to test here the almost-trivial constructors.


I mean, the positive in `Inner::Inner()`, which has regressed, was written long before the notion of "almost-trivial constructor" was introduced by you. So it shouldn't be a problem if we modify this test to let the constructor become non-almost-trivial; that wasn't the purpose of that particular test.

And we could create another test with almost-trivial constructor elsewhere, which would prove the usefulness of your patch.

> > 2. Add a FIXME-test for this checker, in which a completely undefined structure is being copied. Perhaps somebody implements this feature some day.

> 

> 

> So it would be a new function with the same structure, and commented out beginning with a FIXME?


Yeah, that's what i thought - not completely commented out, just having an opposite expected-warning <=> no-warning, and a "FIXME: we should (not) warn here because *some hand-waving*". There are a few such tests around.

> > 3. Leave out the situation that the current test checks as a grey-area. I'm still not convinced that this situation (copying a partially-initialized almost-trivial structure) deserves a warning from this checker.

> 

> 

> This is what I originally did here in the patch. Maybe a FIXME could be written there a well?


I think this test should still test what it used to test - i've a feeling it's ok if we modify the part of it that was not related to what it used to test, but i'm feeling bad about removing it completely.

Essentially, this plan of mine has the sole purpose of keeping my (and perhaps somebody else's) conscience from biting me for removing the test; it seems that simpler approaches seem to carry more of the "no-no, this shouldn't happen" feeling. I guess i could post a patch-over-a-patch if what i'm expressing isn't clear.


https://reviews.llvm.org/D22374