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
