[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
Szelethus closed this revision. Szelethus added a comment. Commited in https://reviews.llvm.org/rC345531. Oops. https://reviews.llvm.org/D53810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > xazax.hun wrote:
> > > > The `raw_ostream` seems to be a common argument. MAybe it would be
> > > > worth to make that a member too?
> > > I did that, and then later changed it back for debugging purposes, if
> > > anyone needs to print note pieces to errs or something.
> > I guess both problems could be solved by doing a private method
> > `raw_ostream` while also wrapping it into a public method with a default
> > `raw_ostream`.
> I think that would add unnecessary complication for very little value. I'd
> rather commit as is.
Sure np, just thinking aloud.
https://reviews.llvm.org/D53810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
NoQ wrote:
> Szelethus wrote:
> > xazax.hun wrote:
> > > The `raw_ostream` seems to be a common argument. MAybe it would be worth
> > > to make that a member too?
> > I did that, and then later changed it back for debugging purposes, if
> > anyone needs to print note pieces to errs or something.
> I guess both problems could be solved by doing a private method `raw_ostream`
> while also wrapping it into a public method with a default `raw_ostream`.
I think that would add unnecessary complication for very little value. I'd
rather commit as is.
https://reviews.llvm.org/D53810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Wow thanks! Great point.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
Szelethus wrote:
> xazax.hun wrote:
> > The `raw_ostream` seems to be a common argument. MAybe it would be worth to
> > make that a member too?
> I did that, and then later changed it back for debugging purposes, if anyone
> needs to print note pieces to errs or something.
I guess both problems could be solved by doing a private method `raw_ostream`
while also wrapping it into a public method with a default `raw_ostream`.
https://reviews.llvm.org/D53810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
xazax.hun wrote:
> The `raw_ostream` seems to be a common argument. MAybe it would be worth to
> make that a member too?
I did that, and then later changed it back for debugging purposes, if anyone
needs to print note pieces to errs or something.
https://reviews.llvm.org/D53810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:82
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
The `raw_ostream` seems to be a common argument. MAybe it would be worth to
make that a member too?
https://reviews.llvm.org/D53810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
Szelethus updated this revision to Diff 171505.
https://reviews.llvm.org/D53810
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -24,10 +24,16 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
+
using namespace clang;
using namespace ento;
using namespace markup;
+//===--===//
+// Declarations of helper classes and functions for emitting bug reports in
+// plist format.
+//===--===//
+
namespace {
class PlistDiagnostics : public PathDiagnosticConsumer {
const std::string OutputFile;
@@ -59,34 +65,91 @@
};
} // end anonymous namespace
-PlistDiagnostics::PlistDiagnostics(AnalyzerOptions ,
- const std::string& output,
- const Preprocessor ,
- bool supportsMultipleFiles)
- : OutputFile(output), PP(PP), AnOpts(AnalyzerOpts),
-SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
+namespace {
-void ento::createPlistDiagnosticConsumer(AnalyzerOptions ,
- PathDiagnosticConsumers ,
- const std::string& s,
- const Preprocessor ) {
- C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
- /*supportsMultipleFiles*/ false));
-}
+/// A helper class for emitting a single report.
+class PlistPrinter {
+ const FIDMap& FM;
+ AnalyzerOptions
+ const Preprocessor
-void ento::createPlistMultiFileDiagnosticConsumer(AnalyzerOptions ,
- PathDiagnosticConsumers ,
- const std::string ,
- const Preprocessor ) {
- C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
- /*supportsMultipleFiles*/ true));
-}
+public:
+ PlistPrinter(const FIDMap& FM, AnalyzerOptions ,
+ const Preprocessor )
+: FM(FM), AnOpts(AnOpts), PP(PP) {
+ }
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
+
+// Don't emit a warning about an unused private field.
+(void)AnOpts;
+ }
+
+private:
+ void ReportPiece(raw_ostream , const PathDiagnosticPiece ,
+ unsigned indent, unsigned depth, bool includeControlFlow,
+ bool isKeyEvent = false) {
+switch (P.getKind()) {
+ case PathDiagnosticPiece::ControlFlow:
+if (includeControlFlow)
+ ReportControlFlow(o, cast(P), indent);
+break;
+ case PathDiagnosticPiece::Call:
+ReportCall(o, cast(P), indent,
+ depth);
+break;
+ case PathDiagnosticPiece::Event:
+ReportEvent(o, cast(P), indent, depth,
+isKeyEvent);
+break;
+ case PathDiagnosticPiece::Macro:
+ReportMacro(o, cast(P), indent, depth);
+break;
+ case PathDiagnosticPiece::Note:
+ReportNote(o, cast(P), indent);
+break;
+}
+ }
+
+ void EmitRanges(raw_ostream , const ArrayRef Ranges,
+ unsigned indent);
+ void EmitMessage(raw_ostream , StringRef Message, unsigned indent);
+
+ void ReportControlFlow(raw_ostream ,
+ const PathDiagnosticControlFlowPiece& P,
+ unsigned indent);
+ void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
+ unsigned indent, unsigned depth, bool isKeyEvent = false);
+ void ReportCall(raw_ostream , const PathDiagnosticCallPiece ,
+ unsigned indent, unsigned depth);
+ void ReportMacro(raw_ostream , const PathDiagnosticMacroPiece& P,
+ unsigned indent, unsigned depth);
+ void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+ unsigned indent);
+};
+
+} // end of anonymous namespace
+
+static void printBugPath(llvm::raw_ostream , const FIDMap& FM,
+ AnalyzerOptions ,
+ const Preprocessor ,
+ const PathPieces );
+
+/// 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,
+
[PATCH] D53810: [analyzer][NFC] Refactor PlistDiagnostics to use a class instead of passing 9 parameters around
Szelethus created this revision.
Szelethus added reviewers: xazax.hun, rnkovacs, george.karpenkov, NoQ.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
This has been a long time coming. Note the usage of `AnalyzerOptions`: I'll
need it for https://reviews.llvm.org/D52742, and added it in
https://reviews.llvm.org/rC343620.
Repository:
rC Clang
https://reviews.llvm.org/D53810
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -24,10 +24,16 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
+
using namespace clang;
using namespace ento;
using namespace markup;
+//===--===//
+// Declarations of helper classes and functions for emitting bug reports in
+// plist format.
+//===--===//
+
namespace {
class PlistDiagnostics : public PathDiagnosticConsumer {
const std::string OutputFile;
@@ -59,34 +65,86 @@
};
} // end anonymous namespace
-PlistDiagnostics::PlistDiagnostics(AnalyzerOptions ,
- const std::string& output,
- const Preprocessor ,
- bool supportsMultipleFiles)
- : OutputFile(output), PP(PP), AnOpts(AnalyzerOpts),
-SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
+namespace {
-void ento::createPlistDiagnosticConsumer(AnalyzerOptions ,
- PathDiagnosticConsumers ,
- const std::string& s,
- const Preprocessor ) {
- C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
- /*supportsMultipleFiles*/ false));
-}
+/// A helper class for emitting a single report.
+class PlistPrinter {
+ const FIDMap& FM;
+ AnalyzerOptions
+ const Preprocessor
-void ento::createPlistMultiFileDiagnosticConsumer(AnalyzerOptions ,
- PathDiagnosticConsumers ,
- const std::string ,
- const Preprocessor ) {
- C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
- /*supportsMultipleFiles*/ true));
-}
+public:
+ PlistPrinter(const FIDMap& FM, AnalyzerOptions ,
+ const Preprocessor )
+: FM(FM), AnOpts(AnOpts), PP(PP) {
+ }
+
+ void ReportDiag(raw_ostream , const PathDiagnosticPiece& P) {
+ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
+
+// Don't emit a warning about an unused private field.
+(void)AnOpts;
+ }
+
+private:
+ void ReportPiece(raw_ostream , const PathDiagnosticPiece ,
+ unsigned indent, unsigned depth, bool includeControlFlow,
+ bool isKeyEvent = false) {
+switch (P.getKind()) {
+ case PathDiagnosticPiece::ControlFlow:
+if (includeControlFlow)
+ ReportControlFlow(o, cast(P), indent);
+break;
+ case PathDiagnosticPiece::Call:
+ReportCall(o, cast(P), indent,
+ depth);
+break;
+ case PathDiagnosticPiece::Event:
+ReportEvent(o, cast(P), indent, depth,
+isKeyEvent);
+break;
+ case PathDiagnosticPiece::Macro:
+ReportMacro(o, cast(P), indent, depth);
+break;
+ case PathDiagnosticPiece::Note:
+ReportNote(o, cast(P), indent);
+break;
+}
+ }
-static void EmitRanges(raw_ostream ,
- const ArrayRef Ranges,
- const FIDMap& FM,
- const Preprocessor ,
- unsigned indent) {
+ void EmitRanges(raw_ostream , const ArrayRef Ranges,
+ unsigned indent);
+ void EmitMessage(raw_ostream , StringRef Message, unsigned indent);
+
+ void ReportControlFlow(raw_ostream ,
+ const PathDiagnosticControlFlowPiece& P,
+ unsigned indent);
+ void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
+ unsigned indent, unsigned depth, bool isKeyEvent = false);
+ void ReportCall(raw_ostream , const PathDiagnosticCallPiece ,
+ unsigned indent, unsigned depth);
+ void ReportMacro(raw_ostream , const PathDiagnosticMacroPiece& P,
+ unsigned indent, unsigned depth);
+ void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+ unsigned indent);
+};
+
+} // end of anonymous namespace
+
+/// Print coverage information to output stream
