NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus,
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet.
Herald added a project: clang.
NoQ added reviewers: alexfh, gribozavr.
NoQ added a parent revision: D67419: [analyzer] NFC: Move PathDiagnostic to
libAnalysis..
The `AnalyzerOptions` object contains too much information that's completely
specific to the Analyzer. It is also being referenced by path diagnostic
consumers to tweak their behavior. If we want path diagnostic consumers to
function separately from the Analyzer, we'll have to make a smaller options
object that only contains relevant options.
@Szelethus: I could have stored `PathDiagnosticConsumerOptions` in
`AnalyzerOptions` by value and pass a const reference around, but it wasn't
pleasant to integrate with `AnalyzerOptions.def`. I.e., i'd have to implement a
new kind of option that doesn't allocate its own field but instead re-uses a
field within a sub-object. Do you want me to go for it or is this
implementation good enough or do you have other approaches in mind?
Repository:
rC Clang
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,16 +65,18 @@
//===----------------------------------------------------------------------===//
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);
+ createPlistMultiFileDiagnosticConsumer(
+ PathDiagnosticConsumerOptions(DiagOpts), C, prefix, PP, CTU);
}
void ento::createTextPathDiagnosticConsumer(
- AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
+ PathDiagnosticConsumerOptions &&DiagOpts, PathDiagnosticConsumers &C,
const std::string &Prefix, const clang::Preprocessor &PP,
const cross_tu::CrossTranslationUnitContext &CTU) {
llvm_unreachable("'text' consumer should be enabled on ClangDiags");
@@ -273,7 +275,7 @@
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"
@@ -418,6 +419,11 @@
///
/// \sa CXXMemberInliningMode
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
+
+ ento::PathDiagnosticConsumerOptions getDiagOpts() const {
+ return {ShouldDisplayMacroExpansions, FullCompilerInvocation,
+ ShouldSerializeStats, ShouldWriteStableReportFilename};
+ }
};
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,30 @@
class PathDiagnostic;
+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 is experimental and needs to be off by default.
+ bool ShouldWriteStableReportFilename;
+};
+
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
