[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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:

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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/

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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;

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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 "

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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 "

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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 ) const { +

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

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: > >