[analyzer] Implement conversion from Clang diagnostics to PathDiagnostics.

[NoQ]

Summary

This clang::DiagnosticConsumer can take normal clang diagnostics (eg., Sema errors or Clang-Tidy warnings) and turn them into static analyzer's PathDiagnostics which are then fed into PathDiagnosticConsumers. It's an essential component for my clang-tidy/static analyzer cross-integration efforts.

The consumer isn't magically aware of information that isn't present in Clang diagnostics but expected in path diagnostics. Such info includes checker name / bug type / category; the consumer expects an external provider to provide such information for every incoming diagnostic.

Additionally, PathDiagnostic's declaration-with-issue and uniqueing-declaration are currently set to nullptr. I'm slowly thinking about auto-detecting those based on the source location but technically all path diagnostic consumer facilities can function just fine without them (declaration-with-issue is there purely for displaying user-visible names and uniqueing declaration only makes sense for path-sensitive warnings that require exotic uniqueing which will probably never be fed into this consumer).

There are also a few features missing here and there, such as fix-it hint support, which i plan to add later.

Though technically possible, this consumer is never supposed to consume the output of TextPathDiagnosticConsumer; a lot of information is lost that way that can't be recovered. In particular, the proper way to teach clang-tidy to emit path diagnostics while keeping in mind that it already ships with static analyzer integration is to allow the integrated static analyzer to talk to path diagnostic consumers directly; static analyzer warnings should never go through this new consumer.

[NoQ]

Add a few comments here and there.

[vsavchenko]

clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h

24

I think that this class (or file?) needs to have some comments on how it is supposed to be used.

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

39

I think this type of stuff should be covered by a comment or a descriptive function name.

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

2

Looks like a copy-paste error :)

64

nit

64

nit: llvm::enumerate or llvm::zip?

84

TBH it is pretty hard to follow the test and from it's structure see what is the input and what is expected output. Maybe it can be reorganized a bit?

140

WE NEED MORE NOUNS!

150–151

Also doesn't seem relevant

155

It is also about the structure, I guess it would've been nice to have all the inputs and expected outputs to be specified here in the actual test, so the reader can figure out what is tested without diving deep into the classes. And it also seems that with the current structure you'll need a couple more classes for every new test.

[steakhal]

Seems pretty straightforward and clean.

The cleanup of the report's message should be reworked.
Besides, that looks good to me.

I think these cases should be tested as well:

• [Warning, Warning, Warning]
• [Warning, Note, Note]
• [Warning, Note, Note, Warning, Note]
• [Warning, Note, Remark, Warning, Remark]

PS: clang-format; sorry for the nit-spam :D

clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h

54–57

Overriding the virtual void clang::DiagnosticConsumer::finish() can't accomplish the same thing?

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

2

I've seen this a few times, and I still don't know why we use it.
The file extension should be enough to recognize that this is a C++ file.
Can someone clarify this?

15

I frequently see #include "clang/..."-ish includes but never #include <clang/...>.

24

38

39

Eh, I don't think it's gonna work this way.
We can not assume that the [ won't appear in the payload of the message.
Eg.:
NewDelete-checker-test.cpp:193

// newdelete-warning{{Argument to 'delete[]' is the address of the local variable 'i', which is not memory allocated by 'new[]'}}

The best you could do is to do a reverse search.
Do we emit the [mypackage.mychecker] suffix for all the reports? If not, then we have a problem.

46–53

So, Error and Fatalis treated in the same way as Warning.
What about the Ignored?

55–61

The ternary operator could simplify this inside the push_back function - emplace_back?

72

Should Desc and ShortDesc be the same?

75–78

Why not construct in place?

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

45

I would suggest removing the redundant virtual. The same applies to the other decls.

64

clang-format should format these files as well, am I right?

84

I can confirm.
What about passing the ExpectedDiags as a parameter?
Probably some shorter type alias could come handy for TestPathDiagnosticConsumer::ExpectedDiagTy and TestPathDiagnosticConsumer::ExpectedPieceTy.

140

What about calling this DiagnosticMock?

[xazax.hun]

clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h

39

A pointer pair might be small enough for a DenseMap?

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

21

Do we need this early return? We might get the same behavior by simply omitting this check. I have no strong preference about keeping or removing it.

[NoQ]

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

2

https://llvm.org/docs/CodingStandards.html#file-headers

(with our file name it doesn't look like there's much space even for a short description)
(i should probably convert the long description into a doxygen comment as well)

15

Whoops thanks.

21

We need it. @steakhal figured this out correctly in the next comment.

39

Uh-oh, mmm, indeed. I should definitely make this optional as well.

55–61

Can't easily use ?: here because LHS and RHS are of different types and operator ?: doesn't do implicit casts.

72

They don't need to be the same but typically clang diagnostics don't provide two different wordings for the same warning.

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

45

Yup, should've said override instead.

155

We have to follow the libTooling architecture where we have to have a FrontendAction class and an ASTConsumer class with specific callback signatures. I'll try to think of something.

[NoQ]

Address review comments!

[NoQ]

[Warning, Note, Remark, Warning, Remark]

I excluded remarks for now with an assertion. Other tests added :)

clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h

54–57

*mind blown*

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

39

Do we emit the [mypackage.mychecker] suffix for all the reports? If not, then we have a problem.

This was supposed to be a workaround for clang-tidy doing this to us. I think it's actually better to teach clang-tidy not to do this when it's pumping output to us. PathDiagnosticConsumers will re-add the checker name when appropriate (eg., in text output but not in plist output).

75–78

With these classes typically there's a bunch of stuff in between, such as Piece->addRange(). Wait a sec, I can actually handle ranges pretty quickly. Let me add them so that this place wasn't empty :D

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

140

We still need to discriminate between FrontendAction mock, ASTConsumer mock, and PathDiagnosticConsumer mock. Dropped some of the nouns though.

[Szelethus]

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

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

69–70

For later, consider testing and handling -werror and -analyzer-werror.

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

140

Do we need really need the postfix DiagnosticConsumer everywhere? Can't we convert it into a namespace? Maybe at least for PathDiagnosticConsumers? Its not a big deal, and it is certainly descriptive, but looks a bit funny.

Also, RIP ClangDiagPathDiagConsumer.

clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h

+//===--- PathDiagnosticConverterDiagnosticConsumer.h ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+///  \file
+///  This file defines a Clang DiagnosticConsumer that converts Clang
+///  diagnostics into PathDiagnostics.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_PATHDIAGNOSTICCONVERTERDIAGNOSTICCONSUMER_H
+#define LLVM_CLANG_ANALYSIS_PATHDIAGNOSTICCONVERTERDIAGNOSTICCONSUMER_H
+
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Analysis/PathDiagnosticConsumers.h"
+
+namespace clang {
+namespace ento {
+
+/// \brief A Clang diagnostic consumer that converts Clang diagnostics into
+/// "path diagnostics" commonly used by the Static Analyzer.
+///
+/// PathDiagnostics are immediately pushed into PathDiagnosticConsumers
+/// that can display clang diagnostics in a number of different ways,
+/// either simply for the visuals or possibly for achieving consistency
+/// with other tools that already use path diagnostics.
+/// If an existing tool already uses a PathDiagnosticConsumer that converts
+/// path diagnostics into clang diagnostics (eg., TextPathDiagnosticConsumer),
+/// putting such diagnostics back into PathDiagnosticConverterDiagnosticConsumer
+/// is undesirable because the original conversion to Clang diagnostics
+/// is not lossless. Instead such tool is supposed to keep accessing
+/// PathDiagnosticConsumers directly. PathDiagnosticConsumers are expected
+/// to be constructed and managed separately.
+class PathDiagnosticConverterDiagnosticConsumer : public DiagnosticConsumer {
+public:
+  struct BugTypeInfo {
+    StringRef CheckName;
+    StringRef BugType;
+    StringRef BugCategory;
+  };
+
+  using BugTypeInfoProvider = std::function<BugTypeInfo(const Diagnostic &)>;
+
+private:
+  PathDiagnosticConsumers &PathConsumers;
+  BugTypeInfoProvider BTIProvider;
+  bool ShouldDisplayNotesAsEvents;
+  bool ShouldTrimCheckerName;
+
+  llvm::DenseMap<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>>
+      PartialPDs;
+
+  void flushPartialDiagnostic();
+
+public:
+  /// Construct a PathDiagnosticConverterDiagnosticConsumer with existing
+  /// PathDiagnosticConsumers and the necessary configuration.
+  /// \param PathConsumers The list of PathDiagnosticConsumers to work with.
+  /// \param BTIProvider A callback that provides additional metadata
+  ///        about each diagnostic that's necessary for path diagnostics
+  ///        but typically not present in regular Clang diagnostics.
+  /// \param ShouldDisplayNotesAsEvents Whether display note diagnostics as path
+  //         events or non-path notes. The latter is always the intended
+  //         behavior as long as the client actually understands non-path notes.
+  /// \param ShouldTrimCheckerName Whether incoming diagnostics are suffixed
+  //         with [checker name] which we can automatically trim.
+  PathDiagnosticConverterDiagnosticConsumer(
+      PathDiagnosticConsumers &PathConsumers, BugTypeInfoProvider &&BTIProvider,
+      bool ShouldDisplayNotesAsEvents, bool ShouldTrimCheckerName)
+      : PathConsumers(PathConsumers), BTIProvider(std::move(BTIProvider)),
+        ShouldDisplayNotesAsEvents(ShouldDisplayNotesAsEvents),
+        ShouldTrimCheckerName(ShouldTrimCheckerName) {}
+
+  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+                        const Diagnostic &Info) override;
+
+  /// Inform the consumer that the last diagnostic has been sent. This is
+  /// necessary because the consumer does not know whether more notes are
+  /// going to be attached to the last warning.
+  void finish() override { flushPartialDiagnostic(); }
+};
+
+} // end 'ento' namespace
+} // end 'clang' namespace
+
+#endif

clang/lib/Analysis/CMakeLists.txt

add_clang_library(clangAnalysis
   CodeInjector.cpp
   Dominators.cpp
   ExprMutationAnalyzer.cpp
   HTMLPathDiagnosticConsumer.cpp
   IssueHash.cpp
   LiveVariables.cpp
   ObjCNoReturn.cpp
   PathDiagnostic.cpp
+  PathDiagnosticConverterDiagnosticConsumer.cpp
   PlistPathDiagnosticConsumer.cpp
   PlistHTMLPathDiagnosticConsumer.cpp
   PostOrderCFGView.cpp
   ProgramPoint.cpp
   ReachableCode.cpp
   RetainSummaryManager.cpp
   SarifPathDiagnosticConsumer.cpp
   TextPathDiagnosticConsumer.cpp

clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

+//===--- PathDiagnosticConverterDiagnosticConsumer.cpp ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the PathDiagnosticConverterDiagnosticConsumer object
+//  which converts regular Clang diagnostics into PathDiagnostics.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h"
+
+using namespace clang;
+using namespace ento;
+
+void PathDiagnosticConverterDiagnosticConsumer::flushPartialDiagnostic() {
+  if (PartialPDs.empty())
+    return;
+
+  assert(PartialPDs.size() == PathConsumers.size() &&
+         "Path diagnostics were constructed for not all consumers");
+
+  for (PathDiagnosticConsumer *Consumer : PathConsumers)
+    Consumer->HandlePathDiagnostic(std::move(PartialPDs[Consumer]));
+
+  PartialPDs.clear();
+}
+
+static void convertDiagnosticData(const Diagnostic &Info,
+                                  PathDiagnosticPiece &Piece) {
+  for (const auto &R : Info.getRanges())
+    Piece.addRange(R.getAsRange());
+
+  // FIXME: Convert more data, eg. fix-it hints.
+}
+
+void PathDiagnosticConverterDiagnosticConsumer::HandleDiagnostic(
+    DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
+  // Count warnings/errors.
+  DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
+
+  llvm::SmallString<128> Msg;
+  Info.FormatDiagnostic(Msg);
+
+  // Auto-capitalize. Path diagnostics are traditionally treated as
+  // full sentences whereas Clang diagnostics are traditionally lowercase.
+  assert(Msg.size() > 0 && "Empty diagnostic message");
+  Msg[0] = std::toupper(Msg[0]);
+
+  // Optionally try to remove checker name.
+  StringRef CleanMsg; // PathDiagnosticPiece will copy it.
+  if (ShouldTrimCheckerName) {
+    size_t I = Msg.str().rfind('[');
+    assert(I != std::string::npos && "Checker name not detected in diagnostic");
+    assert(I > 0 && "Diagnostic message contains only checker name");
+    CleanMsg = Msg.str().substr(0, I - 1);
+  } else {
+    CleanMsg = Msg;
+  }
+
+  SourceLocation Loc = Info.getLocation();
+  const SourceManager &SM = Info.getSourceManager();
+
+  PathDiagnosticLocation PDLoc(Loc, SM);
+
+  // 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
+  // but are fed to us independently. The good news is they always follow their
+  // respective warnings and come in in the right order so we can
+  // automatically re-attach them.
+  if (DiagLevel == DiagnosticsEngine::Note) {
+    for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+      PathDiagnosticPieceRef Piece;
+      if (ShouldDisplayNotesAsEvents) {
+        Piece = std::make_shared<PathDiagnosticEventPiece>(PDLoc, CleanMsg);
+      } else {
+        Piece = std::make_shared<PathDiagnosticNotePiece>(PDLoc, CleanMsg);
+      }
+
+      convertDiagnosticData(Info, *Piece);
+
+      PartialPDs[Consumer]->getMutablePieces().push_back(Piece);
+    }
+  } else {
+    assert((DiagLevel == DiagnosticsEngine::Warning ||
+            DiagLevel == DiagnosticsEngine::Error) &&
+           "Unsupported diagnostic level");
+
+    // No more incoming notes. The current path diagnostic is complete.
+    flushPartialDiagnostic();
+
+    BugTypeInfo BTI = BTIProvider(Info);
+
+    for (PathDiagnosticConsumer *Consumer : PathConsumers) {
+      auto PD = std::make_unique<PathDiagnostic>(
+          BTI.CheckName, /*DeclWithIssue=*/nullptr, BTI.BugType,
+          CleanMsg, CleanMsg, BTI.BugCategory, PDLoc, /*DeclToUnique=*/nullptr,
+          std::make_unique<FilesToLineNumsMap>());
+
+      PathDiagnosticPieceRef Piece =
+          std::make_shared<PathDiagnosticEventPiece>(PDLoc, CleanMsg);
+      convertDiagnosticData(Info, *Piece);
+      PD->setEndOfPath(std::move(Piece));
+
+      PartialPDs[Consumer] = std::move(PD);
+    }
+  }
+}

clang/unittests/Analysis/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_unittest(ClangAnalysisTests
   CFGDominatorTree.cpp
   CFGTest.cpp
   CloneDetectionTest.cpp
   ExprMutationAnalyzerTest.cpp
+  PathDiagnosticConverterDiagnosticConsumer.cpp
   )
 
 clang_target_link_libraries(ClangAnalysisTests
   PRIVATE
   clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangSerialization
   clangTooling
   )

clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp

+//===- PathDiagnosticConverterDiagnosticConsumer.cpp ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file provides unittests for PathDiagnosticConverterDiagnosticConsumer.
+///
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ento;
+using namespace llvm;
+
+struct ExpectedLocationTy {
+  unsigned Line;
+  unsigned Column;
+
+  void testEquality(SourceLocation L, SourceManager &SM) const {
+    EXPECT_EQ(SM.getSpellingLineNumber(L), Line);
+    EXPECT_EQ(SM.getSpellingColumnNumber(L), Column);
+  }
+};
+
+struct ExpectedRangeTy {
+  ExpectedLocationTy Begin;
+  ExpectedLocationTy End;
+
+  void testEquality(SourceRange R, SourceManager &SM) const {
+    Begin.testEquality(R.getBegin(), SM);
+    End.testEquality(R.getEnd(), SM);
+  }
+};
+
+struct ExpectedPieceTy {
+  ExpectedLocationTy Loc;
+  std::string Text;
+  std::vector<ExpectedRangeTy> Ranges;
+
+  void testEquality(const PathDiagnosticPiece &Piece, SourceManager &SM) {
+    Loc.testEquality(Piece.getLocation().asLocation(), SM);
+    EXPECT_EQ(Piece.getString(), Text);
+    EXPECT_EQ(Ranges.size(), Piece.getRanges().size());
+    for (const auto &RangeItem : llvm::enumerate(Piece.getRanges()))
+      Ranges[RangeItem.index()].testEquality(RangeItem.value(), SM);
+  }
+};
+
+struct ExpectedDiagTy {
+  ExpectedLocationTy Loc;
+  std::string VerboseDescription;
+  std::string ShortDescription;
+  std::string CheckerName;
+  std::string BugType;
+  std::string Category;
+  std::vector<ExpectedPieceTy> Path;
+
+  void testEquality(const PathDiagnostic &Diag, SourceManager &SM) {
+    Loc.testEquality(Diag.getLocation().asLocation(), SM);
+    EXPECT_EQ(Diag.getVerboseDescription(), VerboseDescription);
+    EXPECT_EQ(Diag.getShortDescription(), ShortDescription);
+    EXPECT_EQ(Diag.getCheckerName(), CheckerName);
+    EXPECT_EQ(Diag.getBugType(), BugType);
+    EXPECT_EQ(Diag.getCategory(), Category);
+
+    EXPECT_EQ(Path.size(), Diag.path.size());
+    for (const auto &PieceItem : llvm::enumerate(Diag.path))
+      Path[PieceItem.index()].testEquality(*PieceItem.value(), SM);
+  }
+};
+
+using ExpectedDiagsTy = std::vector<ExpectedDiagTy>;
+
+namespace {
+class TestPathDiagnosticConsumer : public PathDiagnosticConsumer {
+  ExpectedDiagsTy ExpectedDiags;
+  SourceManager &SM;
+
+public:
+  TestPathDiagnosticConsumer(ExpectedDiagsTy &&ExpectedDiags, SourceManager &SM)
+      : ExpectedDiags(std::move(ExpectedDiags)), SM(SM) {}
+
+  StringRef getName() const override { return "test"; }
+
+  void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
+                            FilesMade *filesMade) override {
+    EXPECT_EQ(Diags.size(), ExpectedDiags.size());
+    for (const auto &Item : llvm::enumerate(Diags))
+      ExpectedDiags[Item.index()].testEquality(*Item.value(), SM);
+  }
+};
+
+PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo
+bugTypeInfoProvider(const Diagnostic &) {
+  return PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo{
+      "test check name", "test bug type", "test bug category"};
+}
+
+class TestASTConsumer : public ASTConsumer {
+  ExpectedDiagsTy ExpectedDiags;
+  ASTContext &ACtx;
+
+  void performTest(const Decl *D) {
+    if (!D->hasBody())
+      return;
+
+    assert(!ExpectedDiags.empty() && "Consumer invoked more than once!");
+    TestPathDiagnosticConsumer TestPDC{std::move(ExpectedDiags),
+                                       ACtx.getSourceManager()};
+    // Sanitize storage after move so that the above assertion actually worked.
+    ExpectedDiags.clear();
+
+    PathDiagnosticConsumers PathConsumers = {&TestPDC};
+
+    llvm::IntrusiveRefCntPtr<DiagnosticsEngine> DiagEngine =
+        new DiagnosticsEngine(new DiagnosticIDs, new DiagnosticOptions);
+    PathDiagnosticConverterDiagnosticConsumer DiagConsumer(
+        PathConsumers, bugTypeInfoProvider,
+        /*ShouldDisplayNotesAsEvents=*/true, /*ShouldTrimCheckerName=*/false);
+    DiagEngine->setClient(&DiagConsumer, /*ShouldOwnClient=*/false);
+    DiagEngine->setSourceManager(&ACtx.getSourceManager());
+
+    unsigned WarningDiagID = DiagEngine->getCustomDiagID(
+        DiagnosticsEngine::Warning, "bar is called");
+    unsigned NoteDiagID = DiagEngine->getCustomDiagID(
+        DiagnosticsEngine::Note, "with argument at least %0");
+
+    const CompoundStmt *Body = cast<CompoundStmt>(D->getBody());
+    for (const Stmt *S : Body->children()) {
+      const auto *CE = cast<CallExpr>(S);
+      DiagEngine->Report(CE->getBeginLoc(), WarningDiagID)
+          << CE->getSourceRange();
+
+      const auto *Arg = cast<IntegerLiteral>(CE->getArg(0));
+      const llvm::APSInt &IArg = *Arg->getIntegerConstantExpr(ACtx);
+      for (uint64_t I = 0; I < IArg.getZExtValue(); ++I)
+        DiagEngine->Report(Arg->getBeginLoc(), NoteDiagID)
+            << (int)(I + 1) << Arg->getSourceRange();
+    }
+
+    // The last diagnostic must be flushed manually!
+    DiagConsumer.finish();
+
+    PathDiagnosticConsumer::FilesMade FM;
+    TestPDC.FlushDiagnostics(&FM);
+  }
+
+public:
+  TestASTConsumer(CompilerInstance &CI, ExpectedDiagsTy &&ExpectedDiags)
+      : ExpectedDiags(std::move(ExpectedDiags)), ACtx(CI.getASTContext()) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+    for (auto *D : DG)
+      performTest(D);
+
+    return true;
+  }
+};
+
+class TestAction : public ASTFrontendAction {
+  ExpectedDiagsTy ExpectedDiags;
+
+public:
+  TestAction(ExpectedDiagsTy &&ExpectedDiags)
+      : ExpectedDiags(std::move(ExpectedDiags)) {}
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef File) override {
+    return std::make_unique<TestASTConsumer>(CI, std::move(ExpectedDiags));
+  }
+};
+
+TEST(PathDiagnosticConverter, ConsumeWarningsAndNotes) {
+  // The consumer would emit a warning against every call of 'bar()'
+  // and attach as many notes to it as the argument of 'bar()' says.
+  // Expected results are listed and should match actual results
+  // for the test to pass.
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::make_unique<TestAction>(ExpectedDiagsTy{
+          {{3, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{3, 3}, "Bar is called", {{{3, 3}, {3, 8}}}}}},
+          {{4, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{4, 3}, "Bar is called", {{{4, 3}, {4, 8}}}}}},
+          {{5, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{5, 3}, "Bar is called", {{{5, 3}, {5, 8}}}}}}}),
+      "void bar(int);\nvoid foo() {\n  bar(0);\n  bar(0);\n  bar(0);\n}\n",
+      "input.c"));
+
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::make_unique<TestAction>(ExpectedDiagsTy{
+          {{3, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{3, 3}, "Bar is called", {{{3, 3}, {3, 8}}}},
+            {{3, 7}, "With argument at least 1", {{{3, 7}, {3, 7}}}},
+            {{3, 7}, "With argument at least 2", {{{3, 7}, {3, 7}}}}}}}),
+      "void bar(int);\nvoid foo() {\n  bar(2);\n}\n", "input.c"));
+
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::make_unique<TestAction>(ExpectedDiagsTy{
+          {{3, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{3, 3}, "Bar is called", {{{3, 3}, {3, 8}}}},
+            {{3, 7}, "With argument at least 1", {{{3, 7}, {3, 7}}}},
+            {{3, 7}, "With argument at least 2", {{{3, 7}, {3, 7}}}}}},
+          {{4, 3},
+           "Bar is called",
+           "Bar is called",
+           "test check name",
+           "test bug type",
+           "test bug category",
+           {{{4, 3}, "Bar is called", {{{4, 3}, {4, 8}}}},
+            {{4, 7}, "With argument at least 1", {{{4, 7}, {4, 7}}}}}}}),
+      "void bar(int);\nvoid foo() {\n  bar(2);\n  bar(1);\n}\n", "input.c"));
+}
+} // namespace