NoQ updated this revision to Diff 219826. NoQ added a comment. Clean up a tiny bit of dead code.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67420/new/ https://reviews.llvm.org/D67420 Files: clang/include/clang/Analysis/PathDiagnostic.h clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp =================================================================== --- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -65,19 +65,14 @@ //===----------------------------------------------------------------------===// 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, + // Duplicate the DiagOpts object into both consumers. + createHTMLDiagnosticConsumer(PathDiagnosticConsumerOptions(DiagOpts), C, 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"); + createPlistMultiFileDiagnosticConsumer( + PathDiagnosticConsumerOptions(DiagOpts), C, prefix, PP, CTU); } namespace { @@ -86,8 +81,11 @@ bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false; public: - ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag) - : Diag(Diag) {} + ClangDiagPathDiagConsumer(PathDiagnosticConsumerOptions &&DiagOpts, + DiagnosticsEngine &Diag) + : Diag(Diag), IncludePath(DiagOpts.ShouldDisplayPathNotes), + ShouldEmitAsError(DiagOpts.ShouldDisplayWarningsAsErrors), + FixitsAsRemarks(DiagOpts.ShouldEmitFixItHintsAsRemarks) {} ~ClangDiagPathDiagConsumer() override {} StringRef getName() const override { return "ClangDiags"; } @@ -98,10 +96,6 @@ 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 = @@ -168,6 +162,14 @@ }; } // end anonymous namespace +void ento::createTextPathDiagnosticConsumer( + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, + const std::string &Prefix, const clang::Preprocessor &PP, + const cross_tu::CrossTranslationUnitContext &CTU) { + C.push_back( + new ClangDiagPathDiagConsumer(std::move(DiagOpts), PP.getDiagnostics())); +} + //===----------------------------------------------------------------------===// // AnalysisConsumer declaration. //===----------------------------------------------------------------------===// @@ -254,26 +256,21 @@ 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()) { + // Create the text consumer unconditionally. It will display warnings + // on the console even if other consumers display the warning to the user + // in a more sophisticated format. + PathDiagnosticConsumerOptions TextDiagOpts = Opts->getDiagOpts(); + TextDiagOpts.ShouldDisplayPathNotes = (Opts->AnalysisDiagOpt == PD_TEXT); + createTextPathDiagnosticConsumer(std::move(TextDiagOpts), PathConsumers, + OutDir, PP, CTU); + + // Create other consumers if requested. + if (Opts->AnalysisDiagOpt != PD_TEXT && !OutDir.empty()) { switch (Opts->AnalysisDiagOpt) { default: #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" } Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -13,7 +13,6 @@ #include "clang/Analysis/PathDiagnostic.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" @@ -29,7 +28,7 @@ std::string OutputFile; public: - SarifDiagnostics(AnalyzerOptions &, const std::string &Output) + SarifDiagnostics(const std::string &Output) : OutputFile(Output) {} ~SarifDiagnostics() override = default; @@ -44,10 +43,10 @@ } // end anonymous namespace void ento::createSarifDiagnosticConsumer( - AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C, + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, const std::string &Output, const Preprocessor &, const cross_tu::CrossTranslationUnitContext &) { - C.push_back(new SarifDiagnostics(AnalyzerOpts, Output)); + C.push_back(new SarifDiagnostics(Output)); } static StringRef getFileName(const FileEntry &FE) { Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -20,7 +20,6 @@ #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" @@ -39,14 +38,18 @@ 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, const std::string &prefix, - const Preprocessor &PP, + PlistDiagnostics(PathDiagnosticConsumerOptions &&DiagOpts, + const std::string &prefix, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU, bool supportsMultipleFiles); @@ -74,23 +77,19 @@ /// 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. @@ -165,11 +164,6 @@ } // 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, @@ -520,11 +514,37 @@ assert(IndentLevel == InputIndentLevel); } -static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM, - AnalyzerOptions &AnOpts, const Preprocessor &PP, - const cross_tu::CrossTranslationUnitContext &CTU, - const PathPieces &Path) { - PlistPrinter Printer(FM, AnOpts, PP, 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) { +} + +void ento::createPlistDiagnosticConsumer( + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, + const std::string &s, const Preprocessor &PP, + const cross_tu::CrossTranslationUnitContext &CTU) { + C.push_back(new PlistDiagnostics(std::move(DiagOpts), s, PP, CTU, + /*supportsMultipleFiles=*/false)); +} + +void ento::createPlistMultiFileDiagnosticConsumer( + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, + const std::string &s, const Preprocessor &PP, + const cross_tu::CrossTranslationUnitContext &CTU) { + C.push_back(new PlistDiagnostics(std::move(DiagOpts), s, PP, CTU, + /*supportsMultipleFiles=*/true)); +} + +void PlistDiagnostics::printBugPath(llvm::raw_ostream &o, const FIDMap &FM, + const PathPieces &Path) { + PlistPrinter Printer(FM, PP, CTU); assert(std::is_partitioned(Path.begin(), Path.end(), [](const PathDiagnosticPieceRef &E) { return E->getKind() == PathDiagnosticPiece::Note; @@ -557,7 +577,7 @@ o << " </array>\n"; - if (!AnOpts.ShouldDisplayMacroExpansions) + if (!DiagOpts.ShouldDisplayMacroExpansions) return; o << " <key>macro_expansions</key>\n" @@ -566,35 +586,6 @@ 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 &s, const Preprocessor &PP, - const cross_tu::CrossTranslationUnitContext &CTU) { - C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU, - /*supportsMultipleFiles*/ false)); -} - -void ento::createPlistMultiFileDiagnosticConsumer( - AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C, - const std::string &s, const Preprocessor &PP, - const cross_tu::CrossTranslationUnitContext &CTU) { - C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP, CTU, - /*supportsMultipleFiles*/ true)); -} void PlistDiagnostics::FlushDiagnosticsImpl( std::vector<const PathDiagnostic *> &Diags, FilesMade *filesMade) { @@ -669,7 +660,7 @@ 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>"; @@ -793,7 +784,7 @@ 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); Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -23,7 +23,6 @@ #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" @@ -58,19 +57,19 @@ 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, + HTMLDiagnostics(PathDiagnosticConsumerOptions &&DiagOpts, const std::string& prefix, const Preprocessor &pp, bool supportsMultipleFiles) - : Directory(prefix), PP(pp), AnalyzerOpts(AnalyzerOpts), + : DiagOpts(std::move(DiagOpts)), Directory(prefix), PP(pp), SupportsCrossFileDiagnostics(supportsMultipleFiles) {} ~HTMLDiagnostics() override { FlushDiagnostics(nullptr); } @@ -135,17 +134,17 @@ } // namespace void ento::createHTMLDiagnosticConsumer( - AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C, + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, const std::string &prefix, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &) { - C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, true)); + C.push_back(new HTMLDiagnostics(std::move(DiagOpts), prefix, PP, true)); } void ento::createHTMLSingleFileDiagnosticConsumer( - AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C, + PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C, const std::string &prefix, const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &) { - C.push_back(new HTMLDiagnostics(AnalyzerOpts, prefix, PP, false)); + C.push_back(new HTMLDiagnostics(std::move(DiagOpts), prefix, PP, false)); } //===----------------------------------------------------------------------===// @@ -218,7 +217,7 @@ 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)) { @@ -508,7 +507,7 @@ <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"> Index: clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h +++ clang/include/clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h @@ -30,8 +30,9 @@ 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" Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h @@ -14,6 +14,7 @@ #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" @@ -281,7 +282,7 @@ 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). @@ -418,6 +419,16 @@ /// /// \sa CXXMemberInliningMode bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const; + + ento::PathDiagnosticConsumerOptions getDiagOpts() const { + return {ShouldDisplayMacroExpansions, + FullCompilerInvocation, + ShouldSerializeStats, + ShouldWriteStableReportFilename, + AnalyzerWerror, + ShouldEmitFixItHintsAsRemarks, + /*ShouldDisplayPathNotes=*/true}; + } }; using AnalyzerOptionsRef = IntrusiveRefCntPtr<AnalyzerOptions>; Index: clang/include/clang/Analysis/PathDiagnostic.h =================================================================== --- clang/include/clang/Analysis/PathDiagnostic.h +++ clang/include/clang/Analysis/PathDiagnostic.h @@ -58,6 +58,47 @@ class PathDiagnostic; +/// These options tweak the behavior of path diangostic consumers. +/// Most of these options are currently supported by very few consumers. +struct PathDiagnosticConsumerOptions { + /// 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; + + /// 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 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 display warnings as hard errors. + /// Useful for breaking your build when issues are found. + bool ShouldDisplayWarningsAsErrors; + + /// Whether the consumer should display fixits as notes of "remark" severity. + /// Useful for testing fixits themselves. + bool ShouldEmitFixItHintsAsRemarks; + + /// Whether the consumer should display path pieces. If off, only warnings + /// and normal notes are displayed. Useful when another consumer is already + /// displaying path notes, and you only want this consumer to notify the user + /// that the warning was emitted. Also useful for testing. + bool ShouldDisplayPathNotes; +}; + class PathDiagnosticConsumer { public: class PDFileEntry : public llvm::FoldingSetNode {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits