NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: clang.
This patch fixes an accidental redundant `</td>` and disables generation of
variable popups within macro popups due to
https://bugs.llvm.org/show_bug.cgi?id=44782
I also added some tests for our HTML files because those were pretty much
non-existent so far. Given that we can now also test scan-build (i actually
landed D69781 <https://reviews.llvm.org/D69781> yesterday), there are no more
excuses for not writing tests. Test `variable-popups.c` was passing previously,
so it's here just for the sake of finally having a test, while the other two
tests were failing on their respective `CHECK-NOT` directives.
Another interesting thing i did was apply `tidy-html5` to our HTML files. They
seem to satisfy the linter now, and btw that's how i found the first issue
(while trying to reduce the second issue by running `tidy-html5` under
`creduce`).
@Charusso: I didn't manage to understand how `PopUpRanges` get passed around
and why, so i removed them. If you have an example of where it matters, please
send tests :)
Repository:
rC Clang
https://reviews.llvm.org/D73993
Files:
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/test/Analysis/html_diagnostics/td-hotfix.c
clang/test/Analysis/html_diagnostics/variable-popups-2.c
clang/test/Analysis/html_diagnostics/variable-popups.c
Index: clang/test/Analysis/html_diagnostics/variable-popups.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/variable-popups.c
@@ -0,0 +1,23 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN: -analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo2() {
+ int a;
+ int b = 1;
+ if (b)
+ bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// CHECK: <span class='variable'>b
+// CHECK-SAME: <table class='variable_popup'><tbody><tr>
+// CHECK-SAME: <td valign='top'>
+// CHECK-SAME: <div class='PathIndex PathIndexPopUp'>1.1</div>
+// CHECK-SAME: </td>
+// CHECK-SAME: <td>'b' is 1</td>
+// CHECK-SAME: </tr></tbody></table>
+// CHECK-SAME: </span>
Index: clang/test/Analysis/html_diagnostics/variable-popups-2.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/variable-popups-2.c
@@ -0,0 +1,28 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN: -analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+#define MACRO if (b)
+
+void foo2() {
+ int a;
+ int b = 1;
+ MACRO
+ bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// For now we don't emit popups inside macros due to UI limitations.
+// Once we do, we should test it thoroughly.
+
+// CHECK-LABEL: <tr class="codeline" data-linenumber="14">
+// CHECK-NOT: <span class='variable'>
+// CHECK-SAME: <span class='macro'>
+// CHECK-SAME: MACRO
+// CHECK-SAME: <span class='macro_popup'>
+// CHECK-SAME: if (b)
+// CHECK-SAME: </span>
+// CHECK-SAME: </span>
Index: clang/test/Analysis/html_diagnostics/td-hotfix.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/html_diagnostics/td-hotfix.c
@@ -0,0 +1,31 @@
+// RUN: rm -fR %t
+// RUN: mkdir %t
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN: -analyzer-output=html -o %t -verify %s
+// RUN: cat %t/report-*.html | FileCheck %s
+
+void bar(int);
+
+void foo() {
+ int a;
+ bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
+}
+
+// CHECK-LABEL: <div id="EndPath" class="msg msgEvent" style="margin-left:3ex">
+// CHECK-SAME: <table class="msgT">
+// CHECK-SAME: <tr>
+// CHECK-SAME: <td valign="top">
+// CHECK-SAME: <div class="PathIndex PathIndexEvent">2</div>
+// CHECK-SAME: </td>
+// CHECK-SAME: <td>
+// CHECK-SAME: <div class="PathNav">
+// CHECK-SAME: <a href="#Path1" title="Previous event (1)">←</a>
+// CHECK-SAME: </div>
+// CHECK-SAME: </td>
+// CHECK-NOT: </td>
+// CHECK-SAME: <td>
+// CHECK-SAME: 1st function call argument is an uninitialized value
+// CHECK-SAME: </td>
+// CHECK-SAME: </tr>
+// CHECK-SAME: </table>
+// CHECK-SAME: </div>
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -91,8 +91,7 @@
unsigned num);
void HandlePiece(Rewriter &R, FileID BugFileID, const PathDiagnosticPiece &P,
- const std::vector<SourceRange> &PopUpRanges, unsigned num,
- unsigned max);
+ unsigned num, unsigned max);
void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range,
const char *HighlightStart = "<span class=\"mrange\">",
@@ -607,50 +606,31 @@
)<<<";
}
-static void
-HandlePopUpPieceStartTag(Rewriter &R,
- const std::vector<SourceRange> &PopUpRanges) {
- for (const auto &Range : PopUpRanges) {
- html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "",
- "<table class='variable_popup'><tbody>",
- /*IsTokenRange=*/true);
- }
-}
-
-static void HandlePopUpPieceEndTag(Rewriter &R,
- const PathDiagnosticPopUpPiece &Piece,
- std::vector<SourceRange> &PopUpRanges,
- unsigned int LastReportedPieceIndex,
- unsigned int PopUpPieceIndex) {
+static void HandlePopUpPieceTag(Rewriter &R,
+ const PathDiagnosticPopUpPiece &Piece,
+ unsigned int LastReportedPieceIndex,
+ unsigned int PopUpPieceIndex) {
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
SourceRange Range(Piece.getLocation().asRange());
+ if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID())
+ return;
// Write out the path indices with a right arrow and the message as a row.
- Out << "<tr><td valign='top'><div class='PathIndex PathIndexPopUp'>"
+ Out << "<table class='variable_popup'><tbody><tr><td valign='top'>"
+ "<div class='PathIndex PathIndexPopUp'>"
<< LastReportedPieceIndex;
// Also annotate the state transition with extra indices.
Out << '.' << PopUpPieceIndex;
- Out << "</div></td><td>" << Piece.getString() << "</td></tr>";
+ Out << "</div></td><td>" << Piece.getString() << "</td></tr>"
+ << "</tbody></table></span>";
- // If no report made at this range mark the variable and add the end tags.
- if (std::find(PopUpRanges.begin(), PopUpRanges.end(), Range) ==
- PopUpRanges.end()) {
- // Store that we create a report at this range.
- PopUpRanges.push_back(Range);
-
- Out << "</tbody></table></span>";
- html::HighlightRange(R, Range.getBegin(), Range.getEnd(),
- "<span class='variable'>", Buf.c_str(),
- /*IsTokenRange=*/true);
- } else {
- // Otherwise inject just the new row at the end of the range.
- html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", Buf.c_str(),
- /*IsTokenRange=*/true);
- }
+ html::HighlightRange(R, Range.getBegin(), Range.getEnd(),
+ "<span class='variable'>", Buf.c_str(),
+ /*IsTokenRange=*/true);
}
void HTMLDiagnostics::RewriteFile(Rewriter &R,
@@ -674,7 +654,6 @@
std::map<int, int> IndexMap;
// Stores the different ranges where we have reported something.
- std::vector<SourceRange> PopUpRanges;
for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
const auto &Piece = *I->get();
@@ -684,17 +663,26 @@
// This adds diagnostic bubbles, but not navigation.
// Navigation through note pieces would be added later,
// as a separate pass through the piece list.
- HandlePiece(R, FID, Piece, PopUpRanges, NumNotePieces, TotalNotePieces);
+ HandlePiece(R, FID, Piece, NumNotePieces, TotalNotePieces);
--NumNotePieces;
} else {
- HandlePiece(R, FID, Piece, PopUpRanges, NumRegularPieces,
- TotalRegularPieces);
+ HandlePiece(R, FID, Piece, NumRegularPieces, TotalRegularPieces);
--NumRegularPieces;
}
}
+ // Add line numbers, header, footer, etc.
+ html::EscapeText(R, FID);
+ html::AddLineNumbers(R, FID);
+
+ // If we have a preprocessor, relex the file and syntax highlight.
+ // We might not have a preprocessor if we come from a deserialized AST file,
+ // for example.
+ html::SyntaxHighlight(R, FID, PP);
+ html::HighlightMacros(R, FID, PP);
+
// Secondary indexing if we are having multiple pop-ups between two notes.
- // (e.g. [(13) 'a' is 'true']; [(13.1) 'b' is 'false']; [(13.2) 'c' is...)
+ // (e.g. [(13) 'a' is 'true']; [(13.1) 'b' is 'false']; [(13.2) 'c' is...])
NumRegularPieces = TotalRegularPieces;
for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
const auto &Piece = *I->get();
@@ -707,8 +695,7 @@
// This marks the variable, adds the </table> end tag and the message
// (list element) as a row. The <table> start tag will be added after the
// rows has been written out. Note: It stores every different range.
- HandlePopUpPieceEndTag(R, *PopUpP, PopUpRanges, NumRegularPieces,
- PopUpPieceIndex);
+ HandlePopUpPieceTag(R, *PopUpP, NumRegularPieces, PopUpPieceIndex);
if (PopUpPieceIndex > 0)
--IndexMap[NumRegularPieces];
@@ -717,24 +704,10 @@
--NumRegularPieces;
}
}
-
- // Add the <table> start tag of pop-up pieces based on the stored ranges.
- HandlePopUpPieceStartTag(R, PopUpRanges);
-
- // Add line numbers, header, footer, etc.
- html::EscapeText(R, FID);
- html::AddLineNumbers(R, FID);
-
- // If we have a preprocessor, relex the file and syntax highlight.
- // We might not have a preprocessor if we come from a deserialized AST file,
- // for example.
- html::SyntaxHighlight(R, FID, PP);
- html::HighlightMacros(R, FID, PP);
}
void HTMLDiagnostics::HandlePiece(Rewriter &R, FileID BugFileID,
const PathDiagnosticPiece &P,
- const std::vector<SourceRange> &PopUpRanges,
unsigned num, unsigned max) {
// For now, just draw a box above the line in question, and emit the
// warning.
@@ -870,7 +843,7 @@
<< (num - 1)
<< "\" title=\"Previous event ("
<< (num - 1)
- << ")\">←</a></div></td>";
+ << ")\">←</a></div>";
}
os << "</td><td>";
@@ -947,14 +920,8 @@
// Now highlight the ranges.
ArrayRef<SourceRange> Ranges = P.getRanges();
- for (const auto &Range : Ranges) {
- // If we have already highlighted the range as a pop-up there is no work.
- if (std::find(PopUpRanges.begin(), PopUpRanges.end(), Range) !=
- PopUpRanges.end())
- continue;
-
+ for (const auto &Range : Ranges)
HighlightRange(R, LPosInfo.first, Range);
- }
}
static void EmitAlphaCounter(raw_ostream &os, unsigned n) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
