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

[Dmitri Gribenko via Phabricator via cfe-commits]

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/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 Description; }
+
----------------
Thanks for the doc comments! Please use three slashes here and in getShortDescription.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:131
+
+  /// The smallest canonical declaration that contains the bug location.
+  /// This is purely cosmetic; the declaration can be presented to the user
----------------
Thanks for the explanation, just one question -- I don't understand what is meant by "canonical".

The bug can be found in a non-canonical declaration.

```
void f(); // canonical decl

void f() { // non-canonical decl
  *(int*)0 = 0; // bug
}
```


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:141
+  /// uniqueing location coincides with their location. A different uniqueing
+  /// location is primarily used by various leak warnings: the warning is placed
+  /// at the location where the last reference to the leaking resource is
----------------
Replace "is primarily used by" with "For example, leak checker that produces a different primary and uniqueing location. ..."


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66572/new/

https://reviews.llvm.org/D66572