NoQ updated this revision to Diff 219821.
NoQ added a comment.
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.
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"; }
@@ -168,6 +166,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 +260,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
