NoQ marked 5 inline comments as done.
NoQ added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:113
+ // encouraged, but the period at the end of the description is still omitted.
+ StringRef getDescription() const { return Descri
gribozavr accepted this revision.
gribozavr added a comment.
I think this patch is a good improvement, and I don't want to hold it back --
but like we discussed before, and like you wrote on the mailing list, I would
want a more simple API for ClangTidy.
Comment at:
clang/in
Szelethus added a comment.
But, of course, let's wait for @gribozavr to give his blessings as well, I'm
only accepting because removing/changing other parts of the API seems to
deserve a separate revision :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66572/new/
https://reviews.llvm
Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
This patch set the goal of splitting `BugReport` into two different report
kinds, and I think it did that well. Not only that, we drastically improved the
documentation, formalized many
NoQ added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:122
+ /// Get the location on which the report should be uniqued.
+ virtual PathDiagnosticLocation getUniqueingLocation() const {
+return Location;
gri
NoQ marked 2 inline comments as done.
NoQ added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:186
+ /// ranges.
+ void addRange(SourceRange R) {
+assert((R.isValid() || Ranges.empty()) && "Invalid range can only be used "
--
NoQ marked an inline comment as done.
NoQ added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:186
+ /// ranges.
+ void addRange(SourceRange R) {
+assert((R.isValid() || Ranges.empty()) && "Invalid range can only be used "
--
Szelethus added a comment.
Also, thank you @gribozavr for your comments -- its very nice to have someone
review a bigger part of our development interface who is knowledgeable about
Clang, but not the Static Analyzer specifically. Looking at your inlines, these
are very fair criticisms, I found
Szelethus added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:108
+
+ StringRef getDescription() const { return Description; }
+
gribozavr wrote:
> What's the difference between description and short description?
gribozavr added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:174
+ /// This location is used by clients rendering diagnostics.
+ virtual PathDiagnosticLocation getLocation(const SourceManager &SM) const {
+assert(Location.
gribozavr added a comment.
Thank you for the conversation so far! This is not a complete review from me,
but I'm trying to avoid branching in too many directions at once.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:122
+ /// Get the location
NoQ marked an inline comment as done.
NoQ added inline comments.
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:75
/// individual bug reports.
class BugReport : public llvm::ilist_node {
public:
NoQ wrote:
> gribozavr wrote:
> >
12 matches
Mail list logo
