[analyzer] NFC: Separate PathDiagnosticConsumer options from AnalyzerOptions.

[NoQ]

Summary

The AnalyzerOptions object contains too much information that's completely specific to the Analyzer. It is also being referenced by path diagnostic consumers to tweak their behavior. If we want path diagnostic consumers to function separately from the Analyzer, we'll have to make a smaller options object that only contains relevant options.

@Szelethus: I could have stored PathDiagnosticConsumerOptions in AnalyzerOptions by value and pass a const reference around, but it wasn't pleasant to integrate with AnalyzerOptions.def. I.e., i'd have to implement a new kind of option that doesn't allocate its own field but instead re-uses a field within a sub-object. Do you want me to go for it or is this implementation good enough or do you have other approaches in mind?

[gribozavr]

Are you planning to move users of these options -- like PlistDiagnostics -- to libAnalysis?

clang/include/clang/Analysis/PathDiagnostic.h

81

"but for some consumers is experimental" -- parse error. What is experimental?

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

535

I think we need an equal sign for ClangTidy to pick it up: /*foo=*/

[Szelethus]

@Szelethus: I could have stored PathDiagnosticConsumerOptions in AnalyzerOptions by value and pass a const reference around, but it wasn't pleasant to integrate with AnalyzerOptions.def. I.e., i'd have to implement a new kind of option that doesn't allocate its own field but instead re-uses a field within a sub-object. Do you want me to go for it or is this implementation good enough?

I don't really like this direction :^) Imagine how tedious and non-obvious it'll be to add a new pathdiagnostic option. I don't like we're copying this around. There is also a discussion to be had about whether having to specify -analyzer-config to a no longer analyzer specific feature is any good.

or do you have other approaches in mind?

I do! The high level idea is to completely separate pathdiagnostic configs from the AnalyzerOptions, I believe this is more in line with your goal.

• Let's create the equivalent of -analyzer-config, -pathdiagnostic-config, and things like -pathdiagnostic-config-help.
• Avoid getting a parsing error for specifying -analyzer-config macro-expansion instead of -pathdiagnostic-config macro-expansion.
  • Create a field similar to AnalyzerOptions::AnalyzerConfigCmdFlags called AnalyzerOptions::AnalyzerConfigAliases with the 4 configs that we're moving over. In compatibility mode, AnalyzerOptions::isUnknownAnalyzerConfig() shall search in this field as well. In non-compatibility mode, this will result in an error.
• Make PathDiagnosticConsumerOptions field of CompilerInvocation, similar to AnalyzerOptions.
• Your idea of implementing a DIAGNOSTIC_OPTION macro sounds nice.
  • Delete these entries from AnalyzerOptions.def.
  • Create clang/include/Analysis/PathDiagnostics.def, list them there.
  • Generate fields to PathDiagnosticConsumerOptions similar to how AnalyzerOptions does it.
• Parse the options CompilerInvocation.cpp
  • Most of the analyzer config parsing functions can be reused!
  • For these select 4 options that suffer compatibility issues, manually check AnalyzerOptions::ConfigTable.

WDYT? I am happy to help out, I realize this is plenty of code to write, if we go for it.

[NoQ]

In D67420#1666141, @Szelethus wrote:

I do!

Hmm, it sounds like you want to force both Clang frontend and Clang-Tidy to use the same set of flags to control these options (?) Much like -analyzer-config, these flags will have different "style" compared to other flags in the tool. Which is probably not too bad for hidden frontend flags that control the Analyzer because they're anyway set by a separate GUI checkbox, but the inconsistency with other flags would be super glaring in case of Clang-Tidy CLI. Do we really want to go in that direction? I'll be much more comfortable with letting each tool deal with its flags the way it prefers - this doesn't look like a good place for code reuse to me.

[NoQ]

In D67420#1666141, @Szelethus wrote:

• For these select 4 options that suffer compatibility issues, manually check AnalyzerOptions::ConfigTable.

*3 options. ToolInvocation is not really an option that you're ever supposed to adjust manually. Which is one more reason for me to think of this structure as of an implementation detail - an interface through which diagnostic consumers' behavior can be tweaked, regardless of why the tool wants it to be tweaked (because the human told it to or because the tool itself knows better).

[NoQ]

In D67420#1666050, @gribozavr wrote:

Are you planning to move users of these options -- like PlistDiagnostics -- to libAnalysis?

Yup, i was supposed to do that in D67422 :)

[NoQ]

Unforget to do the same for the text consumer. As a side effect, the factory function for the text consumer is no longer special, which will be less confusing when put in libAnalysis.

Address minor comments, don't address major comments.

[NoQ]

Clean up a tiny bit of dead code.

[gribozavr]

PathDiagnosticConsumerOptions is pretty lightweight, and is not passed around in hot paths if I understand correctly. What do you think about passing it by value everywhere?

[Szelethus]

In D67420#1666578, @NoQ wrote:

In D67420#1666141, @Szelethus wrote:

I do!

Hmm, it sounds like you want to force both Clang frontend and Clang-Tidy to use the same set of flags to control these options (?) Much like -analyzer-config, these flags will have different "style" compared to other flags in the tool. Which is probably not too bad for hidden frontend flags that control the Analyzer because they're anyway set by a separate GUI checkbox, but the inconsistency with other flags would be super glaring in case of Clang-Tidy CLI. Do we really want to go in that direction? I'll be much more comfortable with letting each tool deal with its flags the way it prefers - this doesn't look like a good place for code reuse to me.

There are two things I wanted to touch on, but kinda failed to emphasize it. First is the topic of whether -analyzer-config flags are fitting for a feature no longer specific to the analyzer, and the second is similar in nature, but in the actual code -- it doesn't doesn't feel natural to me that AnalyzerOptions is required to construct this, while at the same time we're trying to make diagnostics construction independent of the analyzer. But I really wouldn't like to overengineer this just for the sake of it :^)

On the first, I'm kinda meh, even if we went for it, it would be a separate revision I guess, but the second has me concerned, unless I'm not following something right.

[NoQ]

In D67420#1668099, @Szelethus wrote:

...and the second is similar in nature, but in the actual code -- it doesn't doesn't feel natural to me that AnalyzerOptions is required to construct this, while at the same time we're trying to make diagnostics construction independent of the analyzer.

It's not that AnalyzerOptions is necessary to construct this, it's more like sufficient. PathDiagnosticConsumerOptions is a plain de-encapsulated aggregate and anybody can aggregate-initialize or mutate it. But in the realm of the Analyzer it has "accidentally" turned out that AnalyzerOptions has just the right amount of information to fill it in.

[Szelethus]

In D67420#1668474, @NoQ wrote:

It's not that AnalyzerOptions is necessary to construct this, it's more like sufficient.

*sudden enlightenment*

Ah, okay, right, I'm a dummie, I think I got what's happening here! :) In that case, the idea sounds great. I'll take a second look on the code later (I have a feeling that we should delete the default ctor of PathDiagnosticConsumerOptions, etc), but I'm just a tad too tired atm :)

[Szelethus]

Aha, so every user gets to create their own PathDiagnosticConsumerOptions object, makes sense! There is no interface misconception, because -analyzer-config will only configure what the analyzer would tinket with. I like this patch! If you dont mind, I'd prefer to have one last round with this, but otherwise LGTM.

clang/include/clang/Analysis/PathDiagnostic.h

63

Lets delete the default constructor. It would be an option to generate this with the analyzer secific .def file, but since we have so few users, lets force them to create this object manually :)

[Szelethus]

clang/include/clang/Analysis/PathDiagnostic.h

81

I suspect you copied these from the .def file, did you change the descriptions there too?

[NoQ]

Rebase!! This work was on hiatus but i plan to continue.

Address a review comment.

There are two new path diagnostic options since the last patch that correspond to the optional behaviors of the text consumer that were introduced since last revision:

• ShouldApplyFixIts;
• ShouldDisplayDiagnosticName.

[NoQ]

clang/include/clang/Analysis/PathDiagnostic.h

81

Nope, these were hand-written. I tried to make sure they don't mention any static analyzer specific things.

[NoQ]

@vsavchenko and others just joining: The backstory and the overall plan are in this thread: http://lists.llvm.org/pipermail/cfe-dev/2019-August/063092.html. Continued at http://lists.llvm.org/pipermail/cfe-dev/2019-September/063229.html.

[vsavchenko]

I do like this change even apart from the clang-tidy integration epic. I strongly believe that decoupling (when done right) makes things easier and this change seems like a good first step.

[Szelethus]

I've seen you resurrecting the thread, I'll just take a bit of time to remember what happened in the previous episodes!

[Szelethus]

LGTM! Very well done!

In D67420#2149461, @vsavchenko wrote:

I strongly believe that decoupling (when done right) makes things easier and this change seems like a good first step.

This patch I think categorizes, rather. We could do a much better job at that with these options, I agree, if we switched over to TableGen (D83475#inline-768468). The problem is, its hard to justify the engineering hours for an arguably negligible gain.

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

48–49

Nice.

clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

39–42

Really nice!

[NoQ]

Remove the ShouldDisplayPathNotes option as it never was an AnalyzerOption to begin with and it only makes things worse.

clang/include/clang/Analysis/PathDiagnostic.h

 namespace ento {
 
 //===----------------------------------------------------------------------===//
 // High-level interface for handlers of path-sensitive diagnostics.
 //===----------------------------------------------------------------------===//
 
 class PathDiagnostic;
 
+/// These options tweak the behavior of path diangostic consumers.
+/// Most of these options are currently supported by very few consumers.
+struct PathDiagnosticConsumerOptions {
+  /// Run-line of the tool that produced the diagnostic.
+  /// It can be included with the diagnostic for debugging purposes.
+  std::string ToolInvocation;
+
+  /// Whether to include additional information about macro expansions
+  /// with the diagnostics, because otherwise they can be hard to obtain
+  /// without re-compiling the program under analysis.
+  bool ShouldDisplayMacroExpansions;
+
+  /// Whether to include LLVM statistics of the process in the diagnostic.
+  /// Useful for profiling the tool on large real-world codebases.
+  bool ShouldSerializeStats;
+
+  /// If the consumer intends to produce multiple output files, should it
+  /// use randomly generated file names for these files (with the tiny risk of
+  /// having random collisions) or deterministic human-readable file names
+  /// (with a larger risk of deterministic collisions or invalid characters
+  /// in the file name). We should not really give this choice to the users
+  /// because deterministic mode is always superior when done right, but
+  /// for some consumers this mode is experimental and needs to be
+  /// off by default.
+  bool ShouldWriteStableReportFilename;
+
+  /// Whether the consumer should treat consumed diagnostics as hard errors.
+  /// Useful for breaking your build when issues are found.
+  bool ShouldDisplayWarningsAsErrors;
+
+  /// Whether the consumer should attempt to rewrite the source file
+  /// with fix-it hints attached to the diagnostics it consumes.
+  bool ShouldApplyFixIts;
+
+  /// Whether the consumer should present the name of the entity that emitted
+  /// the diagnostic (eg., a checker) so that the user knew how to disable it.
+  bool ShouldDisplayDiagnosticName;
+
+  PathDiagnosticConsumerOptions() = delete;
+};
+
 class PathDiagnosticConsumer {
 public:
   class PDFileEntry : public llvm::FoldingSetNode {
   public:
     PDFileEntry(llvm::FoldingSetNodeID &NodeID) : NodeID(NodeID) {}
 
     using ConsumerFiles = std::vector<std::pair<StringRef, StringRef>>;
 

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

 //===- AnalyzerOptions.h - Analysis Engine Options --------------*- 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 header defines various options for the static analyzer that are set
 // by the frontend and are consulted throughout the analyzer.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 #define LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
 
+#include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include <string>
 #include <utility>
public:
   unsigned UnoptimizedCFG : 1;
   unsigned PrintStats : 1;
 
   /// Do not re-analyze paths leading to exhausted nodes with a different
   /// strategy. We get better code coverage when retry is enabled.
   unsigned NoRetryExhausted : 1;
 
   /// Emit analyzer warnings as errors.
-  unsigned AnalyzerWerror : 1;
+  bool AnalyzerWerror : 1;
 
   /// The inlining stack depth limit.
   // Cap the stack depth at 4 calls (5 stack frames, base + 4 calls).
   unsigned InlineMaxStackDepth = 5;
 
   /// The mode of function selection used during inlining.
   AnalysisInliningMode InliningMode = NoRedundancy;
 
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
   /// Returns the option controlling which C++ member functions will be
   /// considered for inlining.
   ///
   /// This is controlled by the 'c++-inlining' config option.
   ///
   /// \sa CXXMemberInliningMode
   bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
+
+  ento::PathDiagnosticConsumerOptions getDiagOpts() const {
+    return {FullCompilerInvocation,
+            ShouldDisplayMacroExpansions,
+            ShouldSerializeStats,
+            ShouldWriteStableReportFilename,
+            AnalyzerWerror,
+            ShouldApplyFixIts,
+            ShouldDisplayCheckerNameForText};
+  }
 };
 
 using AnalyzerOptionsRef = IntrusiveRefCntPtr<AnalyzerOptions>;
 
 //===----------------------------------------------------------------------===//
 // We'll use AnalyzerOptions in the frontend, but we can't link the frontend
 // with clangStaticAnalyzerCore, because clangStaticAnalyzerCore depends on
 // clangFrontend.

clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h

 }
 
 namespace ento {
 
 class PathDiagnosticConsumer;
 typedef std::vector<PathDiagnosticConsumer*> PathDiagnosticConsumers;
 
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)                    \
-  void CREATEFN(AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,     \
-                const std::string &Prefix, const Preprocessor &PP,             \
+  void CREATEFN(PathDiagnosticConsumerOptions Diagopts,                        \
+                PathDiagnosticConsumers &C, const std::string &Prefix,         \
+                const Preprocessor &PP,                                        \
                 const cross_tu::CrossTranslationUnitContext &CTU);
 #include "clang/StaticAnalyzer/Core/Analyses.def"
 
 } // end 'ento' namespace
 } // end 'clang' namespace
 
 #endif

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
-#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Errc.h"

 
 //===----------------------------------------------------------------------===//
 // Boilerplate.
 //===----------------------------------------------------------------------===//
 
 namespace {
 
 class HTMLDiagnostics : public PathDiagnosticConsumer {
+  PathDiagnosticConsumerOptions DiagOpts;
   std::string Directory;
   bool createdDir = false;
   bool noDir = false;
   const Preprocessor &PP;
-  AnalyzerOptions &AnalyzerOpts;
   const bool SupportsCrossFileDiagnostics;
 
 public:
-  HTMLDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &OutputDir,
-                  const Preprocessor &pp, bool supportsMultipleFiles)
-      : Directory(OutputDir), PP(pp), AnalyzerOpts(AnalyzerOpts),
+  HTMLDiagnostics(PathDiagnosticConsumerOptions DiagOpts,
+                  const std::string &OutputDir, const Preprocessor &pp,
+                  bool supportsMultipleFiles)
+      : DiagOpts(std::move(DiagOpts)), Directory(OutputDir), PP(pp),
         SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
 
   ~HTMLDiagnostics() override { FlushDiagnostics(nullptr); }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                             FilesMade *filesMade) override;
 
   StringRef getName() const override {
private:
   void dumpCoverageData(const PathDiagnostic &D,
                         const PathPieces &path,
                         llvm::raw_string_ostream &os);
 };
 
 } // namespace
 
 void ento::createHTMLDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     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);
+  createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, CTU);
 
   // TODO: Emit an error here.
   if (OutputDir.empty())
     return;
 
-  C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
+  C.push_back(new HTMLDiagnostics(std::move(DiagOpts), OutputDir, PP, true));
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &OutputDir, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
+  createTextMinimalPathDiagnosticConsumer(DiagOpts, C, OutputDir, PP, 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);
+  C.push_back(new HTMLDiagnostics(std::move(DiagOpts), OutputDir, PP, false));
 }
 
 void ento::createPlistHTMLDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, 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,
+      DiagOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
+      CTU);
+  createPlistMultiFileDiagnosticConsumer(DiagOpts, C, prefix, PP, CTU);
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, 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,
if (report.empty()) {
     llvm::errs() << "warning: no diagnostics generated for main file.\n";
     return;
   }
 
   // Create a path for the target HTML file.
   int FD;
   SmallString<128> Model, ResultPath;
 
-  if (!AnalyzerOpts.ShouldWriteStableReportFilename) {
+  if (!DiagOpts.ShouldWriteStableReportFilename) {
       llvm::sys::path::append(Model, Directory, "report-%%%%%%.html");
       if (std::error_code EC =
           llvm::sys::fs::make_absolute(Model)) {
           llvm::errs() << "warning: could not make '" << Model
                        << "' absolute: " << EC.message() << '\n';
         return;
       }
       if (std::error_code EC =

 </table>
 <!-- REPORTSUMMARYEXTRA -->
 <h3>Annotated Source Code</h3>
 <p>Press <a href="#" onclick="toggleHelp(); return false;">'?'</a>
    to see keyboard shortcuts</p>
 <input type="checkbox" class="spoilerhider" id="showinvocation" />
 <label for="showinvocation" >Show analyzer invocation</label>
 <div class="spoiler">clang -cc1 )<<<";
-    os << html::EscapeText(AnalyzerOpts.FullCompilerInvocation);
+    os << html::EscapeText(DiagOpts.ToolInvocation);
     os << R"<<<(
 </div>
 <div id='tooltiphint' hidden="true">
   <p>Keyboard shortcuts: </p>
   <ul>
     <li>Use 'j/k' keys for keyboard navigation</li>
     <li>Use 'Shift+S' to show/hide relevant lines</li>
     <li>Use '?' to toggle this window</li>

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

 #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"
 #include <memory>
 
 using namespace clang;
 using namespace ento;
 using namespace markup;
 
 //===----------------------------------------------------------------------===//
 // Declarations of helper classes and functions for emitting bug reports in
 // plist format.
 //===----------------------------------------------------------------------===//
 
 namespace {
   class PlistDiagnostics : public PathDiagnosticConsumer {
+    PathDiagnosticConsumerOptions DiagOpts;
     const std::string OutputFile;
     const Preprocessor &PP;
     const cross_tu::CrossTranslationUnitContext &CTU;
-    AnalyzerOptions &AnOpts;
     const bool SupportsCrossFileDiagnostics;
+
+    void printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
+                      const PathPieces &Path);
+
   public:
-    PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
+    PlistDiagnostics(PathDiagnosticConsumerOptions DiagOpts,
                      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;
namespace {
   };
 } // end anonymous namespace
 
 namespace {
 
 /// A helper class for emitting a single report.
 class PlistPrinter {
   const FIDMap& FM;
-  AnalyzerOptions &AnOpts;
   const Preprocessor &PP;
   const cross_tu::CrossTranslationUnitContext &CTU;
   llvm::SmallVector<const PathDiagnosticMacroPiece *, 0> MacroPieces;
 
 public:
-  PlistPrinter(const FIDMap& FM, AnalyzerOptions &AnOpts,
+  PlistPrinter(const FIDMap& FM,
                const Preprocessor &PP,
                const cross_tu::CrossTranslationUnitContext &CTU)
-    : FM(FM), AnOpts(AnOpts), PP(PP), CTU(CTU) {
+    : FM(FM), PP(PP), CTU(CTU) {
   }
 
   void ReportDiag(raw_ostream &o, const PathDiagnosticPiece& P) {
     ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
-
-    // Don't emit a warning about an unused private field.
-    (void)AnOpts;
   }
 
   /// Print the expansions of the collected macro pieces.
   ///
   /// Each time ReportDiag is called on a PathDiagnosticMacroPiece (or, if one
   /// is found through a call piece, etc), it's subpieces are reported, and the
   /// piece itself is collected. Call this function after the entire bugpath
   /// was reported.
struct ExpansionInfo {
   std::string MacroName;
   std::string Expansion;
   ExpansionInfo(std::string N, std::string E)
     : MacroName(std::move(N)), Expansion(std::move(E)) {}
 };
 
 } // end of anonymous namespace
 
-static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM,
-                         AnalyzerOptions &AnOpts, const Preprocessor &PP,
-                         const cross_tu::CrossTranslationUnitContext &CTU,
-                         const PathPieces &Path);
-
 /// Print coverage information to output stream {@code o}.
 /// May modify the used list of files {@code Fids} by inserting new ones.
 static void printCoverage(const PathDiagnostic *D,
                           unsigned InputIndentLevel,
                           SmallVectorImpl<FileID> &Fids,
                           FIDMap &FM,
                           llvm::raw_fd_ostream &o);
 
for (auto I = ExecutedLines.begin(), E = ExecutedLines.end(); I != E; ++I) {
     Indent(o, IndentLevel) << "</array>\n";
   }
   IndentLevel--;
   Indent(o, IndentLevel) << "</dict>\n";
 
   assert(IndentLevel == InputIndentLevel);
 }
 
-static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM,
-                         AnalyzerOptions &AnOpts, const Preprocessor &PP,
-                         const cross_tu::CrossTranslationUnitContext &CTU,
+//===----------------------------------------------------------------------===//
+// Methods of PlistDiagnostics.
+//===----------------------------------------------------------------------===//
+
+PlistDiagnostics::PlistDiagnostics(
+    PathDiagnosticConsumerOptions DiagOpts, const std::string &output,
+    const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU,
+    bool supportsMultipleFiles)
+    : DiagOpts(std::move(DiagOpts)), OutputFile(output), PP(PP), CTU(CTU),
+      SupportsCrossFileDiagnostics(supportsMultipleFiles) {
+  // FIXME: Will be used by a later planned change.
+  (void)this->CTU;
+}
+
+void ento::createPlistDiagnosticConsumer(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &OutputFile, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  // TODO: Emit an error here.
+  if (OutputFile.empty())
+    return;
+
+  C.push_back(new PlistDiagnostics(DiagOpts, OutputFile, PP, CTU,
+                                   /*supportsMultipleFiles=*/false));
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
+                                          PP, CTU);
+}
+
+void ento::createPlistMultiFileDiagnosticConsumer(
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
+    const std::string &OutputFile, const Preprocessor &PP,
+    const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  // TODO: Emit an error here.
+  if (OutputFile.empty())
+    return;
+
+  C.push_back(new PlistDiagnostics(DiagOpts, OutputFile, PP, CTU,
+                                   /*supportsMultipleFiles=*/true));
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, OutputFile,
+                                          PP, CTU);
+}
+
+void PlistDiagnostics::printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
                                     const PathPieces &Path) {
-  PlistPrinter Printer(FM, AnOpts, PP, CTU);
+  PlistPrinter Printer(FM, PP, CTU);
   assert(std::is_partitioned(Path.begin(), Path.end(),
                              [](const PathDiagnosticPieceRef &E) {
                                return E->getKind() == PathDiagnosticPiece::Note;
                              }) &&
          "PathDiagnostic is not partitioned so that notes precede the rest");
 
   PathPieces::const_iterator FirstNonNote = std::partition_point(
       Path.begin(), Path.end(), [](const PathDiagnosticPieceRef &E) {
void PlistDiagnostics::printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
 
   o << "   <array>\n";
 
   for (PathPieces::const_iterator E = Path.end(); I != E; ++I)
     Printer.ReportDiag(o, **I);
 
   o << "   </array>\n";
 
-  if (!AnOpts.ShouldDisplayMacroExpansions)
+  if (!DiagOpts.ShouldDisplayMacroExpansions)
     return;
 
   o << "   <key>macro_expansions</key>\n"
        "   <array>\n";
   Printer.ReportMacroExpansions(o, /* indent */ 4);
   o << "   </array>\n";
 }
 
-//===----------------------------------------------------------------------===//
-// Methods of PlistDiagnostics.
-//===----------------------------------------------------------------------===//
-
-PlistDiagnostics::PlistDiagnostics(
-    AnalyzerOptions &AnalyzerOpts, const std::string &output,
-    const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU,
-    bool supportsMultipleFiles)
-    : 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 &OutputFile, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &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 &OutputFile, const Preprocessor &PP,
-    const cross_tu::CrossTranslationUnitContext &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();
o << " <key>diagnostics</key>\n"
        " <array>\n";
 
   for (std::vector<const PathDiagnostic*>::iterator DI=Diags.begin(),
        DE = Diags.end(); DI!=DE; ++DI) {
 
     o << "  <dict>\n";
 
     const PathDiagnostic *D = *DI;
-    printBugPath(o, FM, AnOpts, PP, CTU, D->path);
+    printBugPath(o, FM, D->path);
 
     // Output the bug type and bug category.
     o << "   <key>description</key>";
     EmitString(o, D->getShortDescription()) << '\n';
     o << "   <key>category</key>";
     EmitString(o, D->getCategory()) << '\n';
     o << "   <key>type</key>";
     EmitString(o, D->getBugType()) << '\n';
void PlistDiagnostics::FlushDiagnosticsImpl(
   o << " </array>\n";
 
   o << " <key>files</key>\n"
        " <array>\n";
   for (FileID FID : Fids)
     EmitString(o << "  ", SM.getFileEntryForID(FID)->getName()) << '\n';
   o << " </array>\n";
 
-  if (llvm::AreStatisticsEnabled() && AnOpts.ShouldSerializeStats) {
+  if (llvm::AreStatisticsEnabled() && DiagOpts.ShouldSerializeStats) {
     o << " <key>statistics</key>\n";
     std::string stats;
     llvm::raw_string_ostream os(stats);
     llvm::PrintStatisticsJSON(os);
     os.flush();
     EmitString(o, html::EscapeText(stats)) << '\n';
   }
 

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

 //===--- SarifDiagnostics.cpp - Sarif 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 SarifDiagnostics object.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/PathDiagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Lex/Preprocessor.h"
-#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Path.h"
 
 using namespace llvm;
 using namespace clang;
 using namespace ento;
 
 namespace {
 class SarifDiagnostics : public PathDiagnosticConsumer {
   std::string OutputFile;
   const LangOptions &LO;
 
 public:
-  SarifDiagnostics(AnalyzerOptions &, const std::string &Output,
-                   const LangOptions &LO)
+  SarifDiagnostics(const std::string &Output, const LangOptions &LO)
       : OutputFile(Output), LO(LO) {}
   ~SarifDiagnostics() override = default;
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                             FilesMade *FM) override;
 
   StringRef getName() const override { return "SarifDiagnostics"; }
   PathGenerationScheme getGenerationScheme() const override { return Minimal; }
   bool supportsLogicalOpControlFlow() const override { return true; }
   bool supportsCrossFileDiagnostics() const override { return true; }
 };
 } // end anonymous namespace
 
 void ento::createSarifDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &Output, const Preprocessor &PP,
     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);
+  C.push_back(new SarifDiagnostics(Output, PP.getLangOpts()));
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), 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

 using namespace ento;
 using namespace tooling;
 
 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 {
+  PathDiagnosticConsumerOptions DiagOpts;
   DiagnosticsEngine &DiagEng;
   const LangOptions &LO;
-  const bool IncludePath = false;
-  const bool ShouldEmitAsError = false;
-  const bool ApplyFixIts = false;
-  const bool ShouldDisplayCheckerName = false;
+  bool ShouldDisplayPathNotes;
 
 public:
-  TextDiagnostics(DiagnosticsEngine &DiagEng, const LangOptions &LO,
-                  bool ShouldIncludePath, const AnalyzerOptions &AnOpts)
-      : DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
-        ShouldEmitAsError(AnOpts.AnalyzerWerror),
-        ApplyFixIts(AnOpts.ShouldApplyFixIts),
-        ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
+  TextDiagnostics(PathDiagnosticConsumerOptions DiagOpts,
+                  DiagnosticsEngine &DiagEng, const LangOptions &LO,
+                  bool ShouldDisplayPathNotes)
+      : DiagOpts(std::move(DiagOpts)), DiagEng(DiagEng), LO(LO),
+        ShouldDisplayPathNotes(ShouldDisplayPathNotes) {}
   ~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;
+    return ShouldDisplayPathNotes ? Minimal : None;
   }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                             FilesMade *filesMade) override {
     unsigned WarnID =
-        ShouldEmitAsError
+        DiagOpts.ShouldDisplayWarningsAsErrors
             ? DiagEng.getCustomDiagID(DiagnosticsEngine::Error, "%0")
             : DiagEng.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
     unsigned NoteID = DiagEng.getCustomDiagID(DiagnosticsEngine::Note, "%0");
     SourceManager &SM = DiagEng.getSourceManager();
 
     Replacements Repls;
     auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
                            ArrayRef<SourceRange> Ranges,
                            ArrayRef<FixItHint> Fixits) {
-      if (!ApplyFixIts) {
+      if (!DiagOpts.ShouldApplyFixIts) {
         DiagEng.Report(Loc, ID) << String << Ranges << Fixits;
         return;
       }
 
       DiagEng.Report(Loc, ID) << String << Ranges;
       for (const FixItHint &Hint : Fixits) {
         Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
 
         if (llvm::Error Err = Repls.add(Repl)) {
           llvm::errs() << "Error applying replacement " << Repl.toString()
                        << ": " << Err << "\n";
         }
       }
     };
 
     for (std::vector<const PathDiagnostic *>::iterator I = Diags.begin(),
          E = Diags.end();
          I != E; ++I) {
       const PathDiagnostic *PD = *I;
-      std::string WarningMsg =
-          (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
+      std::string WarningMsg = (DiagOpts.ShouldDisplayDiagnosticName
+                                    ? " [" + PD->getCheckerName() + "]"
+                                    : "")
                                    .str();
 
       reportPiece(WarnID, PD->getLocation().asLocation(),
                   (PD->getShortDescription() + WarningMsg).str(),
                   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)
+      if (!ShouldDisplayPathNotes)
         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());
       }
     }
 
-    if (!ApplyFixIts || Repls.empty())
+    if (Repls.empty())
       return;
 
     Rewriter Rewrite(SM, LO);
     if (!applyAllReplacements(Repls, Rewrite)) {
       llvm::errs() << "An error occured during applying fix-it.\n";
     }
 
     Rewrite.overwriteChangedFiles();
   }
 };
 } // end anonymous namespace
 
 void ento::createTextPathDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &Prefix, const clang::Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
-  C.emplace_back(new TextDiagnostics(PP.getDiagnostics(), PP.getLangOpts(),
-                                     /*ShouldIncludePath*/ true, AnalyzerOpts));
+  C.emplace_back(new TextDiagnostics(std::move(DiagOpts), PP.getDiagnostics(),
+                                     PP.getLangOpts(),
+                                     /*ShouldDisplayPathNotes=*/true));
 }
 
 void ento::createTextMinimalPathDiagnosticConsumer(
-    AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+    PathDiagnosticConsumerOptions DiagOpts, PathDiagnosticConsumers &C,
     const std::string &Prefix, const clang::Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
-  C.emplace_back(new TextDiagnostics(PP.getDiagnostics(), PP.getLangOpts(),
-                                     /*ShouldIncludePath*/ false,
-                                     AnalyzerOpts));
+  C.emplace_back(new TextDiagnostics(std::move(DiagOpts), PP.getDiagnostics(),
+                                     PP.getLangOpts(),
+                                     /*ShouldDisplayPathNotes=*/false));
 }

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

public:
   }
 
   void DigestAnalyzerOptions() {
     switch (Opts->AnalysisDiagOpt) {
     case PD_NONE:
       break;
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)                    \
   case PD_##NAME:                                                              \
-    CREATEFN(*Opts.get(), PathConsumers, OutDir, PP, CTU);                     \
+    CREATEFN(Opts->getDiagOpts(), PathConsumers, OutDir, PP, CTU);             \
     break;
 #include "clang/StaticAnalyzer/Core/Analyses.def"
     default:
       llvm_unreachable("Unknown analyzer output type!");
     }
 
     // Create the analyzer component creators.
     switch (Opts->AnalysisStoreOpt) {