[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

 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 {}

 
 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()) {}
 

                                          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())

   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),

 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;
 

 
 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;
 

 
   // 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>";

 
 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,

 
 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 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);

 }
 
 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,

   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;
   }
 }

   // 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());
     }
   };
 

     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 << "   <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";
 

     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';
 

           // 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()) {

   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) {