[analyzer][NFC] Refactor functions in PlistDiagnostics to take Preproc as parameter

[Szelethus]

Summary

This is patch is a preparation for the proposed inclusion of macro expansions in the plist output.

Also, would it be okay to clang-format this file?

[george.karpenkov]

Please reupload with context (-U999)

[Szelethus]

Oops.

[george.karpenkov]

lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

98

+1 for identifying redundant arguments.

[Szelethus]

Would you be okay with me clang-formatting this file? I'm aware of a few people doing some work with it, but the diff didn't seem too bad to cause any serious rebasing nightmares.

[george.karpenkov]

Sorry no --- going through those while doing archaeology sucks.
You could reformat your changed lines though.

[Szelethus]

Ah, okay, I'm fine with that.

lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

 #include "llvm/Support/Casting.h"
 using namespace clang;
 using namespace ento;
 using namespace markup;
 
 namespace {
   class PlistDiagnostics : public PathDiagnosticConsumer {
     const std::string OutputFile;
-    const LangOptions &LangOpts;
+    const Preprocessor &PP;
     const bool SupportsCrossFileDiagnostics;
     const bool SerializeStatistics;
   public:
     PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
                      const std::string& prefix,
-                     const LangOptions &LangOpts,
+                     const Preprocessor &PP,
                      bool supportsMultipleFiles);
 
     ~PlistDiagnostics() override {}
 
     void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
                               FilesMade *filesMade) override;
 
     StringRef getName() const override {
       return "PlistDiagnostics";
     }
 
     PathGenerationScheme getGenerationScheme() const override {
       return Extensive;
     }
     bool supportsLogicalOpControlFlow() const override { return true; }
     bool supportsCrossFileDiagnostics() const override {
       return SupportsCrossFileDiagnostics;
     }
   };
 } // end anonymous namespace
 
 PlistDiagnostics::PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
                                    const std::string& output,
-                                   const LangOptions &LO,
+                                   const Preprocessor &PP,
                                    bool supportsMultipleFiles)
   : OutputFile(output),
-    LangOpts(LO),
+    PP(PP),
     SupportsCrossFileDiagnostics(supportsMultipleFiles),
     SerializeStatistics(AnalyzerOpts.shouldSerializeStats()) {}
 
 void ento::createPlistDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
                                          PathDiagnosticConsumers &C,
                                          const std::string& s,
                                          const Preprocessor &PP) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s,
-                                   PP.getLangOpts(), false));
+  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, false));
 }
 
 void ento::createPlistMultiFileDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
                                                   PathDiagnosticConsumers &C,
                                                   const std::string &s,
                                                   const Preprocessor &PP) {
-  C.push_back(new PlistDiagnostics(AnalyzerOpts, s,
-                                   PP.getLangOpts(), true));
+  C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, true));
 }
 
 static void EmitRanges(raw_ostream &o,
                        const ArrayRef<SourceRange> Ranges,
                        const FIDMap& FM,
-                       const SourceManager &SM,
-                       const LangOptions &LangOpts,
+                       const Preprocessor &PP,
                        unsigned indent) {
 
   if (Ranges.empty())
     return;
 
   Indent(o, indent) << "<key>ranges</key>\n";
   Indent(o, indent) << "<array>\n";
   ++indent;
+
+  const SourceManager &SM = PP.getSourceManager();
+  const LangOptions &LangOpts = PP.getLangOpts();
+
   for (auto &R : Ranges)
     EmitRange(o, SM,
               Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
               FM, indent + 1);
   --indent;
   Indent(o, indent) << "</array>\n";
 }
 
static void EmitMessage(raw_ostream &o, StringRef Message, unsigned indent) {
   Indent(o, indent) << "<key>message</key>\n";
   Indent(o, indent);
   EmitString(o, Message) << '\n';
 }
 
 static void ReportControlFlow(raw_ostream &o,
                               const PathDiagnosticControlFlowPiece& P,
                               const FIDMap& FM,
-                              const SourceManager &SM,
-                              const LangOptions &LangOpts,
+                              const Preprocessor &PP,
                               unsigned indent) {
 
+  const SourceManager &SM = PP.getSourceManager();
+  const LangOptions &LangOpts = PP.getLangOpts();
+
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
   Indent(o, indent) << "<key>kind</key><string>control</string>\n";
 
   // Emit edges.
   Indent(o, indent) << "<key>edges</key>\n";
   ++indent;
static void ReportControlFlow(raw_ostream &o,
   }
 
   --indent;
   Indent(o, indent) << "</dict>\n";
 }
 
 static void ReportEvent(raw_ostream &o, const PathDiagnosticEventPiece& P,
                         const FIDMap& FM,
-                        const SourceManager &SM,
-                        const LangOptions &LangOpts,
+                        const Preprocessor &PP,
                         unsigned indent,
                         unsigned depth,
                         bool isKeyEvent = false) {
 
+  const SourceManager &SM = PP.getSourceManager();
+
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
   Indent(o, indent) << "<key>kind</key><string>event</string>\n";
 
   if (isKeyEvent) {
     Indent(o, indent) << "<key>key_event</key><true/>\n";
   }
 
   // Output the location.
   FullSourceLoc L = P.getLocation().asLocation();
 
   Indent(o, indent) << "<key>location</key>\n";
   EmitLocation(o, SM, L, FM, indent);
 
   // Output the ranges (if any).
   ArrayRef<SourceRange> Ranges = P.getRanges();
-  EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
+  EmitRanges(o, Ranges, FM, PP, indent);
 
   // Output the call depth.
   Indent(o, indent) << "<key>depth</key>";
   EmitInteger(o, depth) << '\n';
 
   // Output the text.
   EmitMessage(o, P.getString(), indent);
 
   // Finish up.
   --indent;
   Indent(o, indent); o << "</dict>\n";
 }
 
 static void ReportPiece(raw_ostream &o,
                         const PathDiagnosticPiece &P,
-                        const FIDMap& FM, const SourceManager &SM,
-                        const LangOptions &LangOpts,
+                        const FIDMap& FM, 
+                        const Preprocessor &PP,
                         unsigned indent,
                         unsigned depth,
                         bool includeControlFlow,
                         bool isKeyEvent = false);
 
 static void ReportCall(raw_ostream &o,
                        const PathDiagnosticCallPiece &P,
-                       const FIDMap& FM, const SourceManager &SM,
-                       const LangOptions &LangOpts,
+                       const FIDMap& FM,
+                       const Preprocessor &PP,
                        unsigned indent,
                        unsigned depth) {
 
   if (auto callEnter = P.getCallEnterEvent())
-    ReportPiece(o, *callEnter, FM, SM, LangOpts, indent, depth, true,
-                P.isLastInMainSourceFile());
+    ReportPiece(o, *callEnter, FM, PP, indent, depth,
+                /*includeControlFlow*/ true, P.isLastInMainSourceFile());
 
 
   ++depth;
 
   if (auto callEnterWithinCaller = P.getCallEnterWithinCallerEvent())
-    ReportPiece(o, *callEnterWithinCaller, FM, SM, LangOpts,
-                indent, depth, true);
+    ReportPiece(o, *callEnterWithinCaller, FM, PP, indent, depth,
+                /*includeControlFlow*/ true);
 
   for (PathPieces::const_iterator I = P.path.begin(), E = P.path.end();I!=E;++I)
-    ReportPiece(o, **I, FM, SM, LangOpts, indent, depth, true);
+    ReportPiece(o, **I, FM, PP, indent, depth,
+                /*includeControlFlow*/ true);
 
   --depth;
 
   if (auto callExit = P.getCallExitEvent())
-    ReportPiece(o, *callExit, FM, SM, LangOpts, indent, depth, true);
+    ReportPiece(o, *callExit, FM, PP, indent, depth,
+                /*includeControlFlow*/ true);
 }
 
 static void ReportMacro(raw_ostream &o,
                         const PathDiagnosticMacroPiece& P,
-                        const FIDMap& FM, const SourceManager &SM,
-                        const LangOptions &LangOpts,
+                        const FIDMap& FM,
+                        const Preprocessor &PP,
                         unsigned indent,
                         unsigned depth) {
 
   for (PathPieces::const_iterator I = P.subPieces.begin(), E=P.subPieces.end();
        I!=E; ++I) {
-    ReportPiece(o, **I, FM, SM, LangOpts, indent, depth, false);
+    ReportPiece(o, **I, FM, PP, indent, depth, /*includeControlFlow*/ false);
   }
 }
 
 static void ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
                         const FIDMap& FM,
-                        const SourceManager &SM,
-                        const LangOptions &LangOpts,
+                        const Preprocessor &PP,
                         unsigned indent,
                         unsigned depth) {
 
+  const SourceManager &SM = PP.getSourceManager();
+
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
   // Output the location.
   FullSourceLoc L = P.getLocation().asLocation();
 
   Indent(o, indent) << "<key>location</key>\n";
   EmitLocation(o, SM, L, FM, indent);
 
   // Output the ranges (if any).
   ArrayRef<SourceRange> Ranges = P.getRanges();
-  EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
+  EmitRanges(o, Ranges, FM, PP, indent);
 
   // Output the text.
   EmitMessage(o, P.getString(), indent);
 
   // Finish up.
   --indent;
   Indent(o, indent); o << "</dict>\n";
 }
 
 static void ReportDiag(raw_ostream &o, const PathDiagnosticPiece& P,
-                       const FIDMap& FM, const SourceManager &SM,
-                       const LangOptions &LangOpts) {
-  ReportPiece(o, P, FM, SM, LangOpts, 4, 0, true);
+                       const FIDMap& FM, const Preprocessor &PP) {
+  ReportPiece(o, P, FM, PP, /*indent*/ 4, /*depth*/ 0, /*isKeyEvent*/ true);
 }
 
 static void ReportPiece(raw_ostream &o,
                         const PathDiagnosticPiece &P,
-                        const FIDMap& FM, const SourceManager &SM,
-                        const LangOptions &LangOpts,
+                        const FIDMap& FM,
+                        const Preprocessor &PP,
                         unsigned indent,
                         unsigned depth,
                         bool includeControlFlow,
                         bool isKeyEvent) {
   switch (P.getKind()) {
     case PathDiagnosticPiece::ControlFlow:
       if (includeControlFlow)
-        ReportControlFlow(o, cast<PathDiagnosticControlFlowPiece>(P), FM, SM,
-                          LangOpts, indent);
+        ReportControlFlow(o, cast<PathDiagnosticControlFlowPiece>(P), FM, PP,
+                          indent);
       break;
     case PathDiagnosticPiece::Call:
-      ReportCall(o, cast<PathDiagnosticCallPiece>(P), FM, SM, LangOpts,
-                 indent, depth);
+      ReportCall(o, cast<PathDiagnosticCallPiece>(P), FM, PP, indent, depth);
       break;
     case PathDiagnosticPiece::Event:
-      ReportEvent(o, cast<PathDiagnosticEventPiece>(P), FM, SM, LangOpts,
-                  indent, depth, isKeyEvent);
+      ReportEvent(o, cast<PathDiagnosticEventPiece>(P), FM, PP, indent, depth,
+                  isKeyEvent);
       break;
     case PathDiagnosticPiece::Macro:
-      ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
-                  indent, depth);
+      ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, PP, indent, depth);
       break;
     case PathDiagnosticPiece::Note:
-      ReportNote(o, cast<PathDiagnosticNotePiece>(P), FM, SM, LangOpts,
-                  indent, depth);
+      ReportNote(o, cast<PathDiagnosticNotePiece>(P), FM, PP, indent, depth);
       break;
   }
 }
 
 /// 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,

 
 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 = nullptr;
-
-  if (!Diags.empty())
-    SM = &Diags.front()->path.front()->getLocation().getManager();
+  const SourceManager& SM = PP.getSourceManager();
+  const LangOptions &LangOpts = PP.getLangOpts();
 
-  auto AddPieceFID = [&FM, &Fids, SM](const PathDiagnosticPiece &Piece) {
-    AddFID(FM, Fids, *SM, Piece.getLocation().asLocation());
+  auto AddPieceFID = [&FM, &Fids, &SM](const PathDiagnosticPiece &Piece) {
+    AddFID(FM, Fids, SM, Piece.getLocation().asLocation());
     ArrayRef<SourceRange> Ranges = Piece.getRanges();
     for (const SourceRange &Range : Ranges) {
-      AddFID(FM, Fids, *SM, Range.getBegin());
-      AddFID(FM, Fids, *SM, Range.getEnd());
+      AddFID(FM, Fids, SM, Range.getBegin());
+      AddFID(FM, Fids, SM, Range.getEnd());
     }
   };
 
   for (const PathDiagnostic *D : Diags) {
 
     SmallVector<const PathPieces *, 5> WorkList;
     WorkList.push_back(&D->path);
 
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;
-    const PathPieces &PP = D->path;
+    const PathPieces &Path = D->path;
 
     assert(std::is_partitioned(
-             PP.begin(), PP.end(),
+             Path.begin(), Path.end(),
              [](const std::shared_ptr<PathDiagnosticPiece> &E)
                { return E->getKind() == PathDiagnosticPiece::Note; }) &&
            "PathDiagnostic is not partitioned so that notes precede the rest");
 
     PathPieces::const_iterator FirstNonNote = std::partition_point(
-        PP.begin(), PP.end(),
+        Path.begin(), Path.end(),
         [](const std::shared_ptr<PathDiagnosticPiece> &E)
           { return E->getKind() == PathDiagnosticPiece::Note; });
 
-    PathPieces::const_iterator I = PP.begin();
+    PathPieces::const_iterator I = Path.begin();
 
-    if (FirstNonNote != PP.begin()) {
+    if (FirstNonNote != Path.begin()) {
       o << "   <key>notes</key>\n"
            "   <array>\n";
 
       for (; I != FirstNonNote; ++I)
-        ReportDiag(o, **I, FM, *SM, LangOpts);
+        ReportDiag(o, **I, FM, PP);
 
       o << "   </array>\n";
     }
 
     o << "   <key>path</key>\n";
 
     o << "   <array>\n";
 
-    for (PathPieces::const_iterator E = PP.end(); I != E; ++I)
-      ReportDiag(o, **I, FM, *SM, LangOpts);
+    for (PathPieces::const_iterator E = Path.end(); I != E; ++I)
+      ReportDiag(o, **I, FM, PP);
 
     o << "   </array>\n";
 
     // 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';
     o << "   <key>check_name</key>";
     EmitString(o, D->getCheckName()) << '\n';
 
     o << "   <!-- This hash is experimental and going to change! -->\n";
     o << "   <key>issue_hash_content_of_line_in_context</key>";
     PathDiagnosticLocation UPDLoc = D->getUniqueingLoc();
-    FullSourceLoc L(SM->getExpansionLoc(UPDLoc.isValid()
+    FullSourceLoc L(SM.getExpansionLoc(UPDLoc.isValid()
                                             ? UPDLoc.asLocation()
                                             : D->getLocation().asLocation()),
-                    *SM);
+                    SM);
     const Decl *DeclWithIssue = D->getDeclWithIssue();
-    EmitString(o, GetIssueHash(*SM, L, D->getCheckName(), D->getBugType(),
+    EmitString(o, GetIssueHash(SM, L, D->getCheckName(), D->getBugType(),
                                DeclWithIssue, LangOpts))
         << '\n';
 
     // Output information about the semantic context where
     // the issue occurred.
     if (const Decl *DeclWithIssue = D->getDeclWithIssue()) {
       // FIXME: handle blocks, which have no name.
       if (const NamedDecl *ND = dyn_cast<NamedDecl>(DeclWithIssue)) {
if (const Decl *DeclWithIssue = D->getDeclWithIssue()) {
 
           // If the bug uniqueing location exists, use it for the hash.
           // For example, this ensures that two leaks reported on the same line
           // will have different issue_hashes and that the hash will identify
           // the leak location even after code is added between the allocation
           // site and the end of scope (leak report location).
           if (UPDLoc.isValid()) {
             FullSourceLoc UFunL(
-                SM->getExpansionLoc(
+                SM.getExpansionLoc(
                     D->getUniqueingDecl()->getBody()->getBeginLoc()),
-                *SM);
+                SM);
             o << "  <key>issue_hash_function_offset</key><string>"
               << L.getExpansionLineNumber() - UFunL.getExpansionLineNumber()
               << "</string>\n";
 
           // Otherwise, use the location on which the bug is reported.
           } else {
-            FullSourceLoc FunL(SM->getExpansionLoc(Body->getBeginLoc()), *SM);
+            FullSourceLoc FunL(SM.getExpansionLoc(Body->getBeginLoc()), SM);
             o << "  <key>issue_hash_function_offset</key><string>"
               << L.getExpansionLineNumber() - FunL.getExpansionLineNumber()
               << "</string>\n";
           }
 
         }
       }
     }
 
     // Output the location of the bug.
     o << "  <key>location</key>\n";
-    EmitLocation(o, *SM, D->getLocation().asLocation(), FM, 2);
+    EmitLocation(o, SM, D->getLocation().asLocation(), FM, 2);
 
     // Output the diagnostic to the sub-diagnostic client, if any.
     if (!filesMade->empty()) {
       StringRef lastName;
       PDFileEntry::ConsumerFiles *files = filesMade->getFiles(*D);
       if (files) {
         for (PDFileEntry::ConsumerFiles::const_iterator CI = files->begin(),
                 CE = files->end(); CI != CE; ++CI) {
for (std::vector<const PathDiagnostic*>::iterator DI=Diags.begin(),
     o << "  </dict>\n";
   }
 
   o << " </array>\n";
 
   o << " <key>files</key>\n"
        " <array>\n";
   for (FileID FID : Fids)
-    EmitString(o << "  ", SM->getFileEntryForID(FID)->getName()) << '\n';
+    EmitString(o << "  ", SM.getFileEntryForID(FID)->getName()) << '\n';
   o << " </array>\n";
 
   if (llvm::AreStatisticsEnabled() && SerializeStatistics) {
     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';
   }
 
   // Finish.
   o << "</dict>\n</plist>";
 }