This revision was automatically updated to reflect the committed changes.
Closed by commit rL283092: [analyzer] Extend bug reports with extra notes
(authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D24278?vs=72519&id=73406#toc
Repository:
rL LLVM
https://reviews.llv
zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
I have no further comments.
https://reviews.llvm.org/D24278
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/c
NoQ updated this revision to Diff 72519.
NoQ marked 10 inline comments as done.
NoQ added a comment.
Addressed inline comments. Renamed extra notes to notes.
https://reviews.llvm.org/D24278
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugRepor
NoQ updated this revision to Diff 72480.
NoQ added a comment.
This diff is a part of the original diff that contains core changes without
checker changes (in particular, without tests). No changes were made, no
comments addressed, just removed checker stuff. Uploading it separately so that
it w
dcoughlin added a comment.
This is awesome! I have some minor comments on the dealloc notes. It is also
fine to remove them entirely; we can add them in a later commit.
Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:604
@@ -588,1 +603,3 @@
+addExtraNoteForDe
zaks.anna added a comment.
Thanks!
Looks good overall. Several comments below.
Comment at: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:160
@@ +159,3 @@
+[](const IntrusiveRefCntPtr &p) {
+ return isa(p.get());
+});
--
bcraig added a subscriber: bcraig.
bcraig added a comment.
Neat! I would have liked to have had this for the Excess Padding Checker.
Currently, the padding checker has a very long diagnostic that recommends a new
order for data members. I think a note (or fixit) would be more appropriate,
bu
NoQ added a comment.
Thanks!
I could have split this up into three patches (one for the core and two patches
for the checkers), however that'd mean that the first patch comes without
tests; so i thought that the patch should be self-contained. Was it a bad idea
after all?
Co
a.sidorin added a comment.
This definitely seems to be useful. However, this patch is pretty big. Some of
its parts are not directly related with the feature being introduced (for
example, changes for copypaste/sub-sequences.cpp). Is it possible to split this
patch? Moreover, as I understand, y
v.g.vassilev added a comment.
Thanks for working on this!
On my browser the note "Detected code clone" note appears slightly off the
highlighted range which was a bit confusing to me.
Given my limited understanding of the SA bug reports, this looks good to me.
Comment at: lib
NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin, teemperor,
v.g.vassilev.
NoQ added a subscriber: cfe-commits.
This patch allows injecting extra note-like diagnostics into bug reports, which
are separate from path diagnostics. Previously, we could only
11 matches
Mail list logo
