[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-28 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL370246: [analyzer] Fix analyzer warnings on analyzer. (authored by dergachev, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. Filed the remaining false positive back on myself as https://bugs.llvm.org/show_bug.cgi?id=43135. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66847/new/ https://reviews.llvm.org/D66847 ___ cfe-commits mailing list cf

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. In D66847#1648209 , @Szelethus wrote: > if (isa(D)) { > const auto *FD = dyn_cast(D); // warn: D is known to be a > FunctionDecl, prefer using cast<> > // ... > } > In D66847#1648339

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment. Another common mistake is this: if (A) { const auto *B = dyn_cast_or_null(A); // warn: A is known to be non-null, prefer dyn_cast } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66847/new/ https://reviews.llvm.org/D66847 __

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment. I mean, such code should indeed be refactored to query the custom RTTI only once, but given that it's locally obvious that there's no null dereference in this code, we should not be emitting a warning on it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66847/new/

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ marked an inline comment as done. NoQ added a comment. In D66847#1648209 , @Szelethus wrote: > > if the dyn_ part is really necessary here, then you crash with a null > > dereference a few lines below because you didn't check the result > > Hmm, can w

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. > if the dyn_ part is really necessary here, then you crash with a null > dereference a few lines below because you didn't check the result Hmm, can we detect things like this?: if (i

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 217537. NoQ edited the summary of this revision. NoQ added a comment. Use `auto` in one more place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66847/new/ https://reviews.llvm.org/D66847 Files: clang/include/clang/StaticAnalyzer/Core/PathSensitive/

[PATCH] D66847: [analyzer] Fix analyzer warnings.

2019-08-27 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet. Herald added a project: clang. They ain't gonna fix themselves... wel