This revision was automatically updated to reflect the committed changes.
Closed by commit rC339237: [analyzer][UninitializedObjectChecker] Fixed a false
negative by no longer… (authored by Szelethus, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D48436
Files:
lib/StaticAnal
Szelethus updated this revision to Diff 159687.
Szelethus added a comment.
Added a TODO.
https://reviews.llvm.org/D48436
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/Analysis/cxx-uninitialized-object.cpp
Index: test/Analysis/cxx-uninitialized-object.cpp
==
Szelethus added a comment.
In https://reviews.llvm.org/D48436#1191458, @NoQ wrote:
> Ok, let's commit this and see how to fix it later.
Thanks! ^-^
> 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
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 prope
Szelethus added a comment.
Polite ping :)
https://reviews.llvm.org/D48436
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+ continue;
+
+// If the CurrentObject is a field of OtherObject,
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+ continue;
+
+// If the CurrentObject is a field of OtherObject, it wi
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObject =
+getObjectVal(OtherCtor, Context);
+if (!OtherObject)
+ continue;
+
+// If the CurrentObject is a field of OtherObject,
NoQ added a comment.
Sorry, i'm still lagging with my reviews because there are a lot of them and i
have to balance it with doing actual work. I'll hopefully get to this soon.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:681-689
+Optional OtherObj
Szelethus added a comment.
Polite ping :)
https://reviews.llvm.org/D48436
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+ const LocationContext *LC = Context.getLocationContext();
+ while ((LC = LC->getParent())) {
+
Szelethus wrote:
> NoQ wrote:
> > george.karpenkov wrote
Szelethus added a comment.
> I'll think about that a bit more; it might be worth it to track such deferred
> subregions in a state trait and drain it whenever we pop back to an explicit
> constructor.
There are so many things to consider, like the following case:
struct DynTBase {};
struct
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+ const LocationContext *LC = Context.getLocationContext();
+ while ((LC = LC->getParent())) {
+
george.karpenkov wrote:
> nit: could we have `while (LC)` foll
NoQ added a comment.
> A call to `Derived::Derived()` previously emitted no warnings. However, with
> these changes, a warning is emitted for `Base::a`.
Yep, a heuristic to skip implicit constructor declarations during analysis and
make the first explicit constructor responsible for initializin
Szelethus added a comment.
Polite ping ^-^
https://reviews.llvm.org/D48436
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus added a comment.
In https://reviews.llvm.org/D48436#1144380, @NoQ wrote:
> I think we need to finish our dialog on who's responsible for initialization
> and why do we need to filter constructors at all, cause it's kinda hanging
> (i.e. https://reviews.llvm.org/D45532#inline-422673).
NoQ added a comment.
I think we need to finish our dialog on who's responsible for initialization
and why do we need to filter constructors at all, cause it's kinda hanging
(i.e. https://reviews.llvm.org/D45532#inline-422673).
https://reviews.llvm.org/D48436
george.karpenkov accepted this revision.
george.karpenkov added a comment.
This revision is now accepted and ready to land.
LGTM with a nit.
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:674
+ const LocationContext *LC = Context.getLocationContext();
+
Szelethus updated this revision to Diff 152343.
Szelethus added a comment.
Moved `LC = LC ->getParent()` to the `while` statement's argument to avoid a
potential infinite loop. Whoops :)
https://reviews.llvm.org/D48436
Files:
lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp
test/
Szelethus created this revision.
Szelethus added reviewers: george.karpenkov, NoQ, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet,
whisperity.
As of now, all constructor calls are ignored that are being called by a
constructor. The point of this wa
20 matches
Mail list logo