[PATCH] D94476: [analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[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.
Herald added a subscriber: nullptr.cpp.

LGTM! As a side note, are we aware of anyone that uses short messages instead of the full length one?



================
Comment at: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:69-70
+
+  // For now we assume that every diagnostic is either a warning or a note
+  // logically attached to a previous warning.
+  // Notes do not come in actually attached to their respective warnings
----------------
For later, consider testing and handling `-werror` and `-analyzer-werror`.


================
Comment at: clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp:139
+
+class PathDiagnosticConverterDiagnosticConsumerTestAction
+    : public ASTFrontendAction {
----------------
NoQ wrote:
> steakhal wrote:
> > vsavchenko wrote:
> > > WE NEED MORE NOUNS!
> > What about calling this `DiagnosticMock`?
> We still need to discriminate between `FrontendAction` mock, `ASTConsumer` mock, and `PathDiagnosticConsumer` mock. Dropped some of the nouns though.
Do we need really need the postfix `DiagnosticConsumer` everywhere? Can't we convert it into a namespace? Maybe at least for `PathDiagnosticConsumer`s? Its not a big deal, and it is certainly descriptive, but looks a bit funny.

Also, RIP `ClangDiagPathDiagConsumer`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94476