[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
NoQ added a comment. Yup thanks!~ Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330766: [analyzer] Add support for the note diagnostic
pieces to plist output format. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45407?vs=141505=143802#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45407
Files:
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -84,6 +84,41 @@
PP.getLangOpts(), true));
}
+static void EmitRanges(raw_ostream ,
+ const ArrayRef Ranges,
+ const FIDMap& FM,
+ const SourceManager ,
+ const LangOptions ,
+ unsigned indent) {
+
+ if (Ranges.empty())
+return;
+
+ Indent(o, indent) << "ranges\n";
+ Indent(o, indent) << "\n";
+ ++indent;
+ for (auto : Ranges)
+EmitRange(o, SM,
+ Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
+ FM, indent + 1);
+ --indent;
+ Indent(o, indent) << "\n";
+}
+
+static void EmitMessage(raw_ostream , StringRef Message, unsigned indent) {
+ // Output the text.
+ assert(!Message.empty());
+ Indent(o, indent) << "extended_message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+
+ // Output the short text.
+ // FIXME: Really use a short string.
+ Indent(o, indent) << "message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+}
+
static void ReportControlFlow(raw_ostream ,
const PathDiagnosticControlFlowPiece& P,
const FIDMap& FM,
@@ -138,7 +173,7 @@
Indent(o, indent) << "\n";
}
-static void ReportEvent(raw_ostream , const PathDiagnosticPiece& P,
+static void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
const FIDMap& FM,
const SourceManager ,
const LangOptions ,
@@ -163,34 +198,14 @@
// Output the ranges (if any).
ArrayRef Ranges = P.getRanges();
-
- if (!Ranges.empty()) {
-Indent(o, indent) << "ranges\n";
-Indent(o, indent) << "\n";
-++indent;
-for (auto : Ranges)
- EmitRange(o, SM,
-Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
-FM, indent + 1);
---indent;
-Indent(o, indent) << "\n";
- }
+ EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
// Output the call depth.
Indent(o, indent) << "depth";
EmitInteger(o, depth) << '\n';
// Output the text.
- assert(!P.getString().empty());
- Indent(o, indent) << "extended_message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
-
- // Output the short text.
- // FIXME: Really use a short string.
- Indent(o, indent) << "message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
+ EmitMessage(o, P.getString(), indent);
// Finish up.
--indent;
@@ -246,6 +261,34 @@
}
}
+static void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+const FIDMap& FM,
+const SourceManager ,
+const LangOptions ,
+unsigned indent,
+unsigned depth) {
+
+ Indent(o, indent) << "\n";
+ ++indent;
+
+ // Output the location.
+ FullSourceLoc L = P.getLocation().asLocation();
+
+ Indent(o, indent) << "location\n";
+ EmitLocation(o, SM, L, FM, indent);
+
+ // Output the ranges (if any).
+ ArrayRef Ranges = P.getRanges();
+ EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
+
+ // Output the text.
+ EmitMessage(o, P.getString(), indent);
+
+ // Finish up.
+ --indent;
+ Indent(o, indent); o << "\n";
+}
+
static void ReportDiag(raw_ostream , const PathDiagnosticPiece& P,
const FIDMap& FM, const SourceManager ,
const LangOptions ) {
@@ -271,15 +314,16 @@
indent, depth);
break;
case PathDiagnosticPiece::Event:
- ReportEvent(o, cast(P), FM, SM, LangOpts,
+ ReportEvent(o, cast(P), FM, SM, LangOpts,
indent, depth, isKeyEvent);
break;
case PathDiagnosticPiece::Macro:
ReportMacro(o, cast(P), FM, SM, LangOpts,
indent, depth);
break;
case PathDiagnosticPiece::Note:
- // FIXME: Extend the plist format to support those.
+ ReportNote(o, cast(P), FM, SM, LangOpts,
+ indent, depth);
break;
}
}
@@ -364,15 +408,39 @@
for (std::vector::iterator DI=Diags.begin(),
DE = Diags.end(); DI!=DE;
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330766: [analyzer] Add support for the note diagnostic
pieces to plist output format. (authored by dergachev, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D45407
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/copypaste/plist-diagnostics.cpp
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -84,6 +84,41 @@
PP.getLangOpts(), true));
}
+static void EmitRanges(raw_ostream ,
+ const ArrayRef Ranges,
+ const FIDMap& FM,
+ const SourceManager ,
+ const LangOptions ,
+ unsigned indent) {
+
+ if (Ranges.empty())
+return;
+
+ Indent(o, indent) << "ranges\n";
+ Indent(o, indent) << "\n";
+ ++indent;
+ for (auto : Ranges)
+EmitRange(o, SM,
+ Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
+ FM, indent + 1);
+ --indent;
+ Indent(o, indent) << "\n";
+}
+
+static void EmitMessage(raw_ostream , StringRef Message, unsigned indent) {
+ // Output the text.
+ assert(!Message.empty());
+ Indent(o, indent) << "extended_message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+
+ // Output the short text.
+ // FIXME: Really use a short string.
+ Indent(o, indent) << "message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+}
+
static void ReportControlFlow(raw_ostream ,
const PathDiagnosticControlFlowPiece& P,
const FIDMap& FM,
@@ -138,7 +173,7 @@
Indent(o, indent) << "\n";
}
-static void ReportEvent(raw_ostream , const PathDiagnosticPiece& P,
+static void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
const FIDMap& FM,
const SourceManager ,
const LangOptions ,
@@ -163,34 +198,14 @@
// Output the ranges (if any).
ArrayRef Ranges = P.getRanges();
-
- if (!Ranges.empty()) {
-Indent(o, indent) << "ranges\n";
-Indent(o, indent) << "\n";
-++indent;
-for (auto : Ranges)
- EmitRange(o, SM,
-Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
-FM, indent + 1);
---indent;
-Indent(o, indent) << "\n";
- }
+ EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
// Output the call depth.
Indent(o, indent) << "depth";
EmitInteger(o, depth) << '\n';
// Output the text.
- assert(!P.getString().empty());
- Indent(o, indent) << "extended_message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
-
- // Output the short text.
- // FIXME: Really use a short string.
- Indent(o, indent) << "message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
+ EmitMessage(o, P.getString(), indent);
// Finish up.
--indent;
@@ -246,6 +261,34 @@
}
}
+static void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+const FIDMap& FM,
+const SourceManager ,
+const LangOptions ,
+unsigned indent,
+unsigned depth) {
+
+ Indent(o, indent) << "\n";
+ ++indent;
+
+ // Output the location.
+ FullSourceLoc L = P.getLocation().asLocation();
+
+ Indent(o, indent) << "location\n";
+ EmitLocation(o, SM, L, FM, indent);
+
+ // Output the ranges (if any).
+ ArrayRef Ranges = P.getRanges();
+ EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
+
+ // Output the text.
+ EmitMessage(o, P.getString(), indent);
+
+ // Finish up.
+ --indent;
+ Indent(o, indent); o << "\n";
+}
+
static void ReportDiag(raw_ostream , const PathDiagnosticPiece& P,
const FIDMap& FM, const SourceManager ,
const LangOptions ) {
@@ -271,15 +314,16 @@
indent, depth);
break;
case PathDiagnosticPiece::Event:
- ReportEvent(o, cast(P), FM, SM, LangOpts,
+ ReportEvent(o, cast(P), FM, SM, LangOpts,
indent, depth, isKeyEvent);
break;
case PathDiagnosticPiece::Macro:
ReportMacro(o, cast(P), FM, SM, LangOpts,
indent, depth);
break;
case PathDiagnosticPiece::Note:
- // FIXME: Extend the plist format to support those.
+ ReportNote(o, cast(P), FM, SM, LangOpts,
+ indent, depth);
break;
}
}
@@ -364,15 +408,39 @@
for (std::vector::iterator DI=Diags.begin(),
DE = Diags.end(); DI!=DE; ++DI) {
-o << " \n"
- " path\n";
+o << " \n";
const PathDiagnostic *D = *DI;
+const PathPieces = D->path;
+
+
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. Can someone commit this for me? I don't have a write access. Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Checked this out, seems to be safely ignored for now. The `.plist` format is pretty resistant to this sort of stuff because most APIs to handle it are almost treating it as native arrays/dictionaries, so i guess it should be fine. In https://reviews.llvm.org/D45407#1068724, @whisperity wrote: > @NoQ The problem with emitting notes as events is that we lose the > information that the node was a `note`. How does Xcode behave with these > notes? Does it ignore them, or can read them from the command-line output of > the analyser? Yep, precisely: notes are completely different from path pieces - they shouldn't be connected to the rest of the path via arrows or necessarily have a certain ordering with respect to the path. For now notes are ignored by Xcode; there's no good UI for them implemented. They are also not used by many on-by-default checkers, so they're mostly an experiment for now. Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
whisperity added a comment. @NoQ The problem with emitting notes as events is that we lose the information that the node was a `node`. How does Xcode behave with these notes? Does it ignore them, or can read them from the command-line output of the analyser? Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. I just had a look with `-analyzer-config notes-as-events=true`, and it works (with CodeChecker, at least). I'd still implement a better support for notes, but this is a great workaround for now. Thanks! Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. > Btw, what sort of UI are you trying to make these extra note pieces of mine > work with? I'm also developing a checker: https://reviews.llvm.org/D45532. This checker emits very important information in notes. When I tried testing it with Ericsson's CodeChecker, I realized that notes aren't displayed. I'm planning to extend it so it will be able to. > Was `-analyzer-config notes-as-events=true` of any help? Never knew about the existence of this flag. Woops. I'll only be able to check it out tomorrow, but I'll post my findings here as soon as I have them :) Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
NoQ added a comment. Sorry, overwhelmed a bit, i'll try to get to this as soon as possible. Btw, what sort of UI are you trying to make these extra note pieces of mine work with? Was `-analyzer-config notes-as-events=true` of any help? Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. Did you have some time to check on those programs? :) Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus added a comment. Thanks! Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
NoQ added a comment. The output looks reasonable to me, but we'll need to see if other consumers of the plist output (IDEs that supports the analyzer, such as Xcode) will be able to accept the modified output (at least, will be able to ignore it). I'll have a look. Repository: rC Clang https://reviews.llvm.org/D45407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45407: [StaticAnalyzer] Added notes to the plist output
Szelethus created this revision.
Szelethus added reviewers: dergachev.a, xazax.hun.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a reviewer: george.karpenkov.
Added notes to `-analyzer-output=plist`.
Repository:
rC Clang
https://reviews.llvm.org/D45407
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/copypaste/plist-diagnostics.cpp
Index: test/Analysis/copypaste/plist-diagnostics.cpp
===
--- test/Analysis/copypaste/plist-diagnostics.cpp
+++ test/Analysis/copypaste/plist-diagnostics.cpp
@@ -17,12 +17,39 @@
return b;
}
-// FIXME: This plist output doesn't include the extra note on line 13.
-// It should be updated once the format for extra notes in plists is defined.
-
// CHECK: diagnostics
// CHECK-NEXT:
// CHECK-NEXT:
+// CHECK-NEXT:notes
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT: location
+// CHECK-NEXT:
+// CHECK-NEXT: line13
+// CHECK-NEXT: col28
+// CHECK-NEXT: file0
+// CHECK-NEXT:
+// CHECK-NEXT: ranges
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT: line13
+// CHECK-NEXT: col28
+// CHECK-NEXT: file0
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT: line18
+// CHECK-NEXT: col1
+// CHECK-NEXT: file0
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT:
+// CHECK-NEXT: extended_message
+// CHECK-NEXT: Similar code here
+// CHECK-NEXT: message
+// CHECK-NEXT: Similar code here
+// CHECK-NEXT:
+// CHECK-NEXT:
// CHECK-NEXT:path
// CHECK-NEXT:
// CHECK-NEXT:
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -84,6 +84,41 @@
PP.getLangOpts(), true));
}
+static void EmitRanges(raw_ostream ,
+ const ArrayRef Ranges,
+ const FIDMap& FM,
+ const SourceManager ,
+ const LangOptions ,
+ unsigned indent) {
+
+ if (Ranges.empty())
+return;
+
+ Indent(o, indent) << "ranges\n";
+ Indent(o, indent) << "\n";
+ ++indent;
+ for (auto : Ranges)
+EmitRange(o, SM,
+ Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
+ FM, indent + 1);
+ --indent;
+ Indent(o, indent) << "\n";
+}
+
+static void EmitMessage(raw_ostream , StringRef Message, unsigned indent) {
+ // Output the text.
+ assert(!Message.empty());
+ Indent(o, indent) << "extended_message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+
+ // Output the short text.
+ // FIXME: Really use a short string.
+ Indent(o, indent) << "message\n";
+ Indent(o, indent);
+ EmitString(o, Message) << '\n';
+}
+
static void ReportControlFlow(raw_ostream ,
const PathDiagnosticControlFlowPiece& P,
const FIDMap& FM,
@@ -138,7 +173,7 @@
Indent(o, indent) << "\n";
}
-static void ReportEvent(raw_ostream , const PathDiagnosticPiece& P,
+static void ReportEvent(raw_ostream , const PathDiagnosticEventPiece& P,
const FIDMap& FM,
const SourceManager ,
const LangOptions ,
@@ -163,34 +198,14 @@
// Output the ranges (if any).
ArrayRef Ranges = P.getRanges();
-
- if (!Ranges.empty()) {
-Indent(o, indent) << "ranges\n";
-Indent(o, indent) << "\n";
-++indent;
-for (auto : Ranges)
- EmitRange(o, SM,
-Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
-FM, indent + 1);
---indent;
-Indent(o, indent) << "\n";
- }
+ EmitRanges(o, Ranges, FM, SM, LangOpts, indent);
// Output the call depth.
Indent(o, indent) << "depth";
EmitInteger(o, depth) << '\n';
// Output the text.
- assert(!P.getString().empty());
- Indent(o, indent) << "extended_message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
-
- // Output the short text.
- // FIXME: Really use a short string.
- Indent(o, indent) << "message\n";
- Indent(o, indent);
- EmitString(o, P.getString()) << '\n';
+ EmitMessage(o, P.getString(), indent);
// Finish up.
--indent;
@@ -246,6 +261,34 @@
}
}
+static void ReportNote(raw_ostream , const PathDiagnosticNotePiece& P,
+const FIDMap& FM,
+const SourceManager ,
+const LangOptions ,
+unsigned indent,
+unsigned depth) {
+
+ Indent(o, indent) << "\n";
+ ++indent;
+
+ // Output the location.
+ FullSourceLoc L =
