[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Ok, let's commit this and see how to fix it later. I still think it's more important to come up with clear rules of who is responsible for initializing fields than making sure our warnings are properly grouped together.

> ideally we wouldn't like to have a warning for an object (`t`) and a separate warning for it's field (`t.bptr.x`)

I don't quite understand this. How would the first warning look like? What would it warn about?



================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:669-671
+  Optional<nonloc::LazyCompoundVal> CurrentObject = getObjectVal(Ctor, Context);
+  if (!CurrentObject)
+    return false;
----------------
All uses of `getObjectVal` so far are followed by retrieving the parent region from the `LazyCompoundVal`. Why do you need to obtain the `LazyCompoundVal` in the first place, i.e. do the second `getSVal` "for '*this'" in `getObjectVal`? Why not just operate over the this-region on the current Store? I think there isn't even a guarantee that these two regions are the same. Like, in this case they probably will be the same, but we shouldn't rely on that.


https://reviews.llvm.org/D48436