[analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

[Szelethus]

Summary

TableGen and .def files (which are meant to be used with the preprocessor) come with obvious downsides. One of those issues is that generated switch-case branches have to be identical. This pushes corner cases either to an outer code block, or into the generated code.

Inspect the removed code in AnalysisConsumer::DigestAnalyzerOptions. You can see how corner cases like a not existing output file, the analysis output type being set to PD_NONE, or whether to complement the output with additional diagnostics on stderr lay around the preprocessor generated code. This is a bit problematic, as to how to deal with such errors is not in the hands of the users of this interface (those implementing output types, like PlistDiagnostics etc).

This patch changes this by moving these corner cases into the generated code, more specifically, into the called functions. In addition, I introduced a new output type for convenience purposes, PD_TEXT_MINIMAL, which always existed conceptually, but never in the actual Analyses.def file. This refactoring allowed me to move TextDiagnostics (renamed from ClangDiagPathDiagConsumer) to its own file, which it really deserved.

Also, those that had the misfortune to gaze upon Analyses.def will probably enjoy the sight that a clang-format did on it.

[Szelethus]

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

148

Will need to handle PD_NONE, or clang-tidy will crash all over the place.

[Szelethus]

Unbreak clang-tidy.

[martong]

clang/include/clang/StaticAnalyzer/Core/Analyses.def

17–18

Yay! I am a big fun of the 80 col limit! :)

61

Just out of curiosity: is there a way to know which one is the default?

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

140

Actually, now I wonder where should we document the default output type ... I am pretty sure it should be done somewhere where it is obvious, maybe in Analyses.def?

142

Man, I understand you pain now, this is really muddled, do you plan to sort this out in an upcoming patch?

148

Would be an assert better here?

148

// if the output
 // directory isn't specified, it acts like if it was in the minimal text
 // output mode.

So, why don't we do this check before calling createTextMinimalPathDiagnosticConsumer ?

clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

2

Is this just a blind copy (and rename) from AnalysisConsumer, or should I take a deeper look into anything here?

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

154–155

Some build bots will not compile if you handle all cases in the swtich and you still have a default, beware.

346

Either the ; is redundant or this is not the end of a namespace.

[Szelethus]

Thanks, @martong!

clang/include/clang/StaticAnalyzer/Core/Analyses.def

61

Not from this file, unfortunately. You need to go to AnalyzerOptions.h. It would be great if we were able to list the possible options as well as the default value (similarly to -analyzer-configs), but I don't have plant to do it myself anytime soon.

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

140

AnalyzerOptions.h.

142

Allow me to indulge you with the relieving fix found in D76510 :)

148

if (OutputDir.empty())
  return;

createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));

Otherwise this would be a functional and breaking change. Mind that in this patch, HTML is still our default output type. If I were to move the text diagnostics below this line, we would no longer emit any diagnostics to stderr. I actually had the same idea, got ~670 breaking test :^)

if (OutputDir.empty()) {
  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
  return;
}

C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));

With this code, we would no longer emit to stderr, only to HTML, should an output directory be specified.

148

Would be an assert better here?

No, the point of the patch is to handle user errors (such as setting the output type to html but forgetting to supply the output location) in these factory functions. An assert would've been appropriate if I left AnalysisConsumer::DigestAnalyzerOptions untouched, as previously, that is where we checked such inconsistencies.

clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

2

Well, almost:

• Moved the initialization of the boolean fields into the constructor (these were previously done with setters in AnalysisConsumer::DigestAnalyzerOptions)
• Renamed the DiagnosticsEngine & field from Diag to DiagEng (because this entire file is about diagnostics, duh)
• Instead of IncludePath being set according to the value of Opts->AnalysisDiagOpt == PD_TEXT, it depends on whether createTextMinimalPathDiagnosticConsumer or createTextPathDiagnosticConsumer is called. But as to which of these is called, of course, depends on Opts->AnalysisDiagOpt == PD_TEXT.

This is such an essential part of the analyzer, almost every test file implicitly checks this code as well, so I don't think there's much to worry about here :)

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

154–155

Oh, thanks!

346

clangd >.<

[martong]

OK, LGTM.

[baloghadamsoftware]

Much more logical than the existing setup. LGTM.

clang/include/clang/StaticAnalyzer/Core/Analyses.def

 //===-- Analyses.def - Metadata about Static Analyses -----------*- 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 set of static analyses used by AnalysisConsumer.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef ANALYSIS_STORE
 #define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store", CreateRegionStoreManager)
+ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store",
+               CreateRegionStoreManager)
 
 #ifndef ANALYSIS_CONSTRAINTS
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of concrete value ranges", CreateRangeConstraintManager)
-ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", CreateZ3ConstraintManager)
+ANALYSIS_CONSTRAINTS(RangeConstraints, "range",
+                     "Use constraint tracking of concrete value ranges",
+                     CreateRangeConstraintManager)
+
+ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver",
+                     CreateZ3ConstraintManager)
 
 #ifndef ANALYSIS_DIAGNOSTICS
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
 #endif
 
-ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML", createHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "html-single-file", "Output analysis results using HTML (not allowing for multi-file bugs)", createHTMLSingleFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", createPlistDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis results using Plists (allowing for multi-file bugs)", createPlistMultiFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF file", createSarifDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", createTextPathDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
+                     createHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+    HTML_SINGLE_FILE, "html-single-file",
+    "Output analysis results using HTML (not allowing for multi-file bugs)",
+    createHTMLSingleFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists",
+                     createPlistDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+    PLIST_MULTI_FILE, "plist-multi-file",
+    "Output analysis results using Plists (allowing for multi-file bugs)",
+    createPlistMultiFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html",
+                     "Output analysis results using HTML wrapped with Plists",
+                     createPlistHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF file",
+                     createSarifDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results to stderr",
+                     createTextPathDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(TEXT_MINIMAL, "text-minimal",
+                     "Emits minimal diagnostics to stderr, stating only the "
+                     "warning message and the associated notes. Usually "
+                     "used in addition to other analysis types",
+                     createTextMinimalPathDiagnosticConsumer)
 
 #ifndef ANALYSIS_PURGE
 #define ANALYSIS_PURGE(NAME, CMDFLAG, DESC)
 #endif
 
-ANALYSIS_PURGE(PurgeStmt,  "statement", "Purge symbols, bindings, and constraints before every statement")
-ANALYSIS_PURGE(PurgeBlock, "block", "Purge symbols, bindings, and constraints before every basic block")
-ANALYSIS_PURGE(PurgeNone,  "none", "Do not purge symbols, bindings, or constraints")
+ANALYSIS_PURGE(
+    PurgeStmt, "statement",
+    "Purge symbols, bindings, and constraints before every statement")
+
+ANALYSIS_PURGE(
+    PurgeBlock, "block",
+    "Purge symbols, bindings, and constraints before every basic block")
+
+ANALYSIS_PURGE(PurgeNone, "none",
+               "Do not purge symbols, bindings, or constraints")
 
 #ifndef ANALYSIS_INLINING_MODE
 #define ANALYSIS_INLINING_MODE(NAME, CMDFLAG, DESC)
 #endif
 
-ANALYSIS_INLINING_MODE(All,  "all", "Analyze all functions as top level")
-ANALYSIS_INLINING_MODE(NoRedundancy, "noredundancy", "Do not analyze a function which has been previously inlined")
+ANALYSIS_INLINING_MODE(All, "all", "Analyze all functions as top level")
+
+ANALYSIS_INLINING_MODE(
+    NoRedundancy, "noredundancy",
+    "Do not analyze a function which has been previously inlined")
 
 #undef ANALYSIS_STORE
 #undef ANALYSIS_CONSTRAINTS
 #undef ANALYSIS_DIAGNOSTICS
 #undef ANALYSIS_PURGE
 #undef ANALYSIS_INLINING_MODE
 #undef ANALYSIS_IPA
-

clang/lib/StaticAnalyzer/Core/CMakeLists.txt

add_clang_library(clangStaticAnalyzerCore
   SimpleConstraintManager.cpp
   SimpleSValBuilder.cpp
   SMTConstraintManager.cpp
   Store.cpp
   SubEngine.cpp
   SValBuilder.cpp
   SVals.cpp
   SymbolManager.cpp
+  TextDiagnostics.cpp
   WorkList.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangAnalysis
   clangBasic
   clangCrossTU
   clangFrontend
   clangLex
   clangRewrite
   )
 

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

class HTMLDiagnostics : public PathDiagnosticConsumer {
   std::string Directory;
   bool createdDir = false;
   bool noDir = false;
   const Preprocessor &PP;
   AnalyzerOptions &AnalyzerOpts;
   const bool SupportsCrossFileDiagnostics;
 
 public:
-  HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts,
-                  const std::string& prefix,
-                  const Preprocessor &pp,
-                  bool supportsMultipleFiles)
-      : Directory(prefix), PP(pp), AnalyzerOpts(AnalyzerOpts),
+  HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &OutputDir,
+                  const Preprocessor &pp, bool supportsMultipleFiles)
+      : Directory(OutputDir), PP(pp), AnalyzerOpts(AnalyzerOpts),
         SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
 
   ~HTMLDiagnostics() override { FlushDiagnostics(nullptr); }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                             FilesMade *filesMade) override;
 
   StringRef getName() const override {
void dumpCoverageData(const PathDiagnostic &D,
                         const PathPieces &path,
                         llvm::raw_string_ostream &os);
 };
 
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
-    const std::string &prefix, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &) {
-  C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, true));
+    const std::string &OutputDir, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  // FIXME: HTML is currently our default output type, but if the output
+  // directory isn't specified, it acts like if it was in the minimal text
+  // output mode. This doesn't make much sense, we should have the minimal text
+  // as our default. In the case of backward compatibility concerns, this could
+  // be preserved with -analyzer-config-compatibility-mode=true.
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
+
+  // TODO: Emit an error here.
+  if (OutputDir.empty())
+    return;
+
+  C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    const std::string &OutputDir, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  // TODO: Emit an error here.
+  if (OutputDir.empty())
+    return;
+
+  C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, false));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
+}
+
+void ento::createPlistHTMLDiagnosticConsumer(
+    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
     const std::string &prefix, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &) {
-  C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, false));
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+  createHTMLDiagnosticConsumer(
+      AnalyzerOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
+      CTU);
+  createPlistMultiFileDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
 }
 
 //===----------------------------------------------------------------------===//
 // Report processing.
 //===----------------------------------------------------------------------===//
 
 void HTMLDiagnostics::FlushDiagnosticsImpl(
   std::vector<const PathDiagnostic *> &Diags,

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

 namespace {
   class PlistDiagnostics : public PathDiagnosticConsumer {
     const std::string OutputFile;
     const Preprocessor &PP;
     const cross_tu::CrossTranslationUnitContext &CTU;
     AnalyzerOptions &AnOpts;
     const bool SupportsCrossFileDiagnostics;
   public:
-    PlistDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &prefix,
-                     const Preprocessor &PP,
+    PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
+                     const std::string &OutputFile, const Preprocessor &PP,
                      const cross_tu::CrossTranslationUnitContext &CTU,
                      bool supportsMultipleFiles);
 
     ~PlistDiagnostics() override {}
 
     void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                               FilesMade *filesMade) override;
 
PlistDiagnostics::PlistDiagnostics(
     : OutputFile(output), PP(PP), CTU(CTU), AnOpts(AnalyzerOpts),
       SupportsCrossFileDiagnostics(supportsMultipleFiles) {
   // FIXME: Will be used by a later planned change.
   (void)this->CTU;
 }
 
 void ento::createPlistDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
-    const std::string &s, const Preprocessor &PP,
+    const std::string &OutputFile, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU,
+
+  // TODO: Emit an error here.
+  if (OutputFile.empty())
+    return;
+
+  C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
                                    /*supportsMultipleFiles*/ false));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputFile, PP, CTU);
 }
 
 void ento::createPlistMultiFileDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
-    const std::string &s, const Preprocessor &PP,
+    const std::string &OutputFile, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU,
+
+  // TODO: Emit an error here.
+  if (OutputFile.empty())
+    return;
+
+  C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
                                    /*supportsMultipleFiles*/ true));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputFile, PP, CTU);
 }
+
 void PlistDiagnostics::FlushDiagnosticsImpl(
                                     std::vector<const PathDiagnostic *> &Diags,
                                     FilesMade *filesMade) {
   // Build up a set of FIDs that we use by scanning the locations and
   // ranges of the diagnostics.
   FIDMap FM;
   SmallVector<FileID, 10> Fids;
   const SourceManager& SM = PP.getSourceManager();

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

public:
   bool supportsLogicalOpControlFlow() const override { return true; }
   bool supportsCrossFileDiagnostics() const override { return true; }
 };
 } // end anonymous namespace
 
 void ento::createSarifDiagnosticConsumer(
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
     const std::string &Output, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &) {
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  // TODO: Emit an error here.
+  if (Output.empty())
+    return;
+
   C.push_back(new SarifDiagnostics(AnalyzerOpts, Output, PP.getLangOpts()));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, Output, PP, CTU);
 }
 
 static StringRef getFileName(const FileEntry &FE) {
   StringRef Filename = FE.tryGetRealPathName();
   if (Filename.empty())
     Filename = FE.getName();
   return Filename;
 }

clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

+//===--- TextDiagnostics.cpp - Text Diagnostics for Paths -------*- 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 TextDiagnostics object.
+//
+//===----------------------------------------------------------------------===//
+//
+#include "clang/Analysis/PathDiagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/PlistSupport.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Version.h"
+#include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/TokenConcatenation.h"
+#include "clang/Rewrite/Core/HTMLRewrite.h"
+#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/IssueHash.h"
+#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
+#include "llvm/Support/Casting.h"
+
+using namespace clang;
+using namespace ento;
+namespace {
+
+/// Emitsd minimal diagnostics (report message + notes) for the 'none' output
+/// type to the standard error, or to to compliment many others. Emits detailed
+/// diagnostics in textual format for the 'text' output type.
+class TextDiagnostics : public PathDiagnosticConsumer {
+  DiagnosticsEngine &DiagEng;
+  const bool IncludePath = false, ShouldEmitAsError = false,
+             FixitsAsRemarks = false;
+
+public:
+  TextDiagnostics(DiagnosticsEngine &Diag, bool ShouldIncludePath,
+                  const AnalyzerOptions &AnOpts)
+      : DiagEng(Diag), IncludePath(ShouldIncludePath),
+        ShouldEmitAsError(AnOpts.AnalyzerWerror),
+        FixitsAsRemarks(AnOpts.ShouldEmitFixItHintsAsRemarks) {}
+  ~TextDiagnostics() override {}
+
+  StringRef getName() const override { return "TextDiagnostics"; }
+
+  bool supportsLogicalOpControlFlow() const override { return true; }
+  bool supportsCrossFileDiagnostics() const override { return true; }
+
+  PathGenerationScheme getGenerationScheme() const override {
+    return IncludePath ? Minimal : None;
+  }
+
+  void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
+                            FilesMade *filesMade) override {
+    unsigned WarnID =
+        ShouldEmitAsError
+            ? DiagEng.getCustomDiagID(DiagnosticsEngine::Error, "%0")
+            : DiagEng.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
+    unsigned NoteID = DiagEng.getCustomDiagID(DiagnosticsEngine::Note, "%0");
+    unsigned RemarkID =
+        DiagEng.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
+
+    auto reportPiece = [&](unsigned ID, SourceLocation Loc, StringRef String,
+                           ArrayRef<SourceRange> Ranges,
+                           ArrayRef<FixItHint> Fixits) {
+      if (!FixitsAsRemarks) {
+        DiagEng.Report(Loc, ID) << String << Ranges << Fixits;
+      } else {
+        DiagEng.Report(Loc, ID) << String << Ranges;
+        for (const FixItHint &Hint : Fixits) {
+          SourceManager &SM = DiagEng.getSourceManager();
+          llvm::SmallString<128> Str;
+          llvm::raw_svector_ostream OS(Str);
+          // FIXME: Add support for InsertFromRange and
+          // BeforePreviousInsertion.
+          assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
+          assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
+          OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) << "-"
+             << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) << ": '"
+             << Hint.CodeToInsert << "'";
+          DiagEng.Report(Loc, RemarkID) << OS.str();
+        }
+      }
+    };
+
+    for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
+                                                       E = Diags.end();
+         I != E; ++I) {
+      const PathDiagnostic *PD = *I;
+      reportPiece(WarnID, PD->getLocation().asLocation(),
+                  PD->getShortDescription(), PD->path.back()->getRanges(),
+                  PD->path.back()->getFixits());
+
+      // First, add extra notes, even if paths should not be included.
+      for (const auto &Piece : PD->path) {
+        if (!isa<PathDiagnosticNotePiece>(Piece.get()))
+          continue;
+
+        reportPiece(NoteID, Piece->getLocation().asLocation(),
+                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+      }
+
+      if (!IncludePath)
+        continue;
+
+      // Then, add the path notes if necessary.
+      PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
+      for (const auto &Piece : FlatPath) {
+        if (isa<PathDiagnosticNotePiece>(Piece.get()))
+          continue;
+
+        reportPiece(NoteID, Piece->getLocation().asLocation(),
+                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+      }
+    }
+  }
+};
+
+} // namespace
+
+void ento::createTextPathDiagnosticConsumer(
+    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    const std::string &Prefix, const clang::Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+  C.emplace_back(new TextDiagnostics(PP.getDiagnostics(),
+                                     /*ShouldIncludePath*/ true, AnalyzerOpts));
+}
+
+void ento::createTextMinimalPathDiagnosticConsumer(
+    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    const std::string &Prefix, const clang::Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+  C.emplace_back(new TextDiagnostics(PP.getDiagnostics(),
+                                     /*ShouldIncludePath*/ false,
+                                     AnalyzerOpts));
+}

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

 STATISTIC(NumBlocksInAnalyzedFunctions,
                       "The # of basic blocks in the analyzed functions.");
 STATISTIC(NumVisitedBlocksInAnalyzedFunctions,
           "The # of visited basic blocks in the analyzed functions.");
 STATISTIC(PercentReachableBlocks, "The % of reachable basic blocks.");
 STATISTIC(MaxCFGSize, "The maximum number of basic blocks in a function.");
 
 //===----------------------------------------------------------------------===//
-// Special PathDiagnosticConsumers.
-//===----------------------------------------------------------------------===//
-
-void ento::createPlistHTMLDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
-    const std::string &prefix, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &CTU) {
-  createHTMLDiagnosticConsumer(
-      AnalyzerOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
-      CTU);
-  createPlistMultiFileDiagnosticConsumer(AnalyzerOpts, C, prefix, PP, CTU);
-}
-
-void ento::createTextPathDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
-    const std::string &Prefix, const clang::Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &CTU) {
-  llvm_unreachable("'text' consumer should be enabled on ClangDiags");
-}
-
-namespace {
-class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
-  DiagnosticsEngine &Diag;
-  bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false;
-
-public:
-  ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag)
-      : Diag(Diag) {}
-  ~ClangDiagPathDiagConsumer() override {}
-  StringRef getName() const override { return "ClangDiags"; }
-
-  bool supportsLogicalOpControlFlow() const override { return true; }
-  bool supportsCrossFileDiagnostics() const override { return true; }
-
-  PathGenerationScheme getGenerationScheme() const override {
-    return IncludePath ? Minimal : None;
-  }
-
-  void enablePaths() { IncludePath = true; }
-  void enableWerror() { ShouldEmitAsError = true; }
-  void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
-
-  void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
-                            FilesMade *filesMade) override {
-    unsigned WarnID =
-        ShouldEmitAsError
-            ? Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0")
-            : Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
-    unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
-    unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
-
-    auto reportPiece =
-        [&](unsigned ID, SourceLocation Loc, StringRef String,
-            ArrayRef<SourceRange> Ranges, ArrayRef<FixItHint> Fixits) {
-          if (!FixitsAsRemarks) {
-            Diag.Report(Loc, ID) << String << Ranges << Fixits;
-          } else {
-            Diag.Report(Loc, ID) << String << Ranges;
-            for (const FixItHint &Hint : Fixits) {
-              SourceManager &SM = Diag.getSourceManager();
-              llvm::SmallString<128> Str;
-              llvm::raw_svector_ostream OS(Str);
-              // FIXME: Add support for InsertFromRange and
-              // BeforePreviousInsertion.
-              assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
-              assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
-              OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin())
-                 << "-" << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd())
-                 << ": '" << Hint.CodeToInsert << "'";
-              Diag.Report(Loc, RemarkID) << OS.str();
-            }
-          }
-        };
-
-    for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
-         E = Diags.end();
-         I != E; ++I) {
-      const PathDiagnostic *PD = *I;
-      reportPiece(WarnID, PD->getLocation().asLocation(),
-                  PD->getShortDescription(), PD->path.back()->getRanges(),
-                  PD->path.back()->getFixits());
-
-      // First, add extra notes, even if paths should not be included.
-      for (const auto &Piece : PD->path) {
-        if (!isa<PathDiagnosticNotePiece>(Piece.get()))
-          continue;
-
-        reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
-      }
-
-      if (!IncludePath)
-        continue;
-
-      // Then, add the path notes if necessary.
-      PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
-      for (const auto &Piece : FlatPath) {
-        if (isa<PathDiagnosticNotePiece>(Piece.get()))
-          continue;
-
-        reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
-      }
-    }
-  }
-};
-} // end anonymous namespace
-
-//===----------------------------------------------------------------------===//
 // AnalysisConsumer declaration.
 //===----------------------------------------------------------------------===//
 
 namespace {
 
 class AnalysisConsumer : public AnalysisASTConsumer,
                          public RecursiveASTVisitor<AnalysisConsumer> {
   enum {
public:
 
   ~AnalysisConsumer() override {
     if (Opts->PrintStats) {
       llvm::PrintStatistics();
     }
   }
 
   void DigestAnalyzerOptions() {
-    if (Opts->AnalysisDiagOpt != PD_NONE) {
-      // Create the PathDiagnosticConsumer.
-      ClangDiagPathDiagConsumer *clangDiags =
-          new ClangDiagPathDiagConsumer(PP.getDiagnostics());
-      PathConsumers.push_back(clangDiags);
-
-      if (Opts->AnalyzerWerror)
-        clangDiags->enableWerror();
-
-      if (Opts->ShouldEmitFixItHintsAsRemarks)
-        clangDiags->enableFixitsAsRemarks();
-
-      if (Opts->AnalysisDiagOpt == PD_TEXT) {
-        clangDiags->enablePaths();
-
-      } else if (!OutDir.empty()) {
-        switch (Opts->AnalysisDiagOpt) {
-        default:
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)                    \
   case PD_##NAME:                                                              \
     CREATEFN(*Opts.get(), PathConsumers, OutDir, PP, CTU);                     \
     break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
-        }
-      }
+    default:
+      llvm_unreachable("Unkown analyzer output type!");
     }
 
     // Create the analyzer component creators.
     switch (Opts->AnalysisStoreOpt) {
     default:
       llvm_unreachable("Unknown store manager.");
 #define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATEFN)           \
       case NAME##Model: CreateStoreMgr = CREATEFN; break;
void DisplayFunction(const Decl *D, AnalysisMode Mode,
     if (Loc.isValid()) {
       llvm::errs() << "ANALYZE";
 
       if (Mode == AM_Syntax)
         llvm::errs() << " (Syntax)";
       else if (Mode == AM_Path) {
         llvm::errs() << " (Path, ";
         switch (IMode) {
-          case ExprEngine::Inline_Minimal:
-            llvm::errs() << " Inline_Minimal";
-            break;
-          case ExprEngine::Inline_Regular:
-            llvm::errs() << " Inline_Regular";
-            break;
         }
         llvm::errs() << ")";
-      }
-      else
+      } else
         assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
 
-      llvm::errs() << ": " << Loc.getFilename() << ' '
-                           << getFunctionName(D) << '\n';
+      llvm::errs() << ": " << Loc.getFilename() << ' ' << getFunctionName(D)
+                   << '\n';
     }
   }
 
   void Initialize(ASTContext &Context) override {
     Ctx = &Context;
     checkerMgr = createCheckerManager(
         *Ctx, *Opts, Plugins, CheckerRegistrationFns, PP.getDiagnostics());
 
private:
   std::string getFunctionName(const Decl *D);
 
   /// Check if we should skip (not analyze) the given function.
   AnalysisMode getModeForDecl(Decl *D, AnalysisMode Mode);
   void runAnalysisOnTranslationUnit(ASTContext &C);
 
   /// Print \p S to stderr if \c Opts->AnalyzerDisplayProgress is set.
   void reportAnalyzerProgress(StringRef S);
-};
+}; // namespace
 } // end anonymous namespace
 
 
 //===----------------------------------------------------------------------===//
 // AnalysisConsumer implementation.
 //===----------------------------------------------------------------------===//
 bool AnalysisConsumer::HandleTopLevelDecl(DeclGroupRef DG) {
   storeTopLevelDecls(DG);