[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks for the fix! The `PopUpRanges` is very important, please revert it back in its original shape. Sorry for the inconvenience.

I have ran a quick scan-build with this patch on LLVM because I wanted to give you a real world example (which you cannot visibly see at the test file). Here it is:
F11294494: Screenshot_20200204_225522.png <https://reviews.llvm.org/F11294494>

The idea of ranges was that to add a new entry into a pop-up note as a table so you could inject any kind of pop-up information in order on the same range. Also if we highlighted the range as a pop-up do not highlight it in gray as well (which could break the HTML). Back in the days I did not realize the importance of the Doxygen and test files, I did not really know how to do so. Here I have adjusted the comments a little-bit:

  + /// Create the pop-up notes' table's start tag.
  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);
    }
  }
  
  + /// Inject a new entry into the pop-up notes' table or add the table's end tag.
  static void HandlePopUpPieceEndTag(Rewriter &R,
                                     const PathDiagnosticPopUpPiece &Piece,
                                     std::vector<SourceRange> &PopUpRanges,
                                     unsigned int LastReportedPieceIndex,
                                     unsigned int PopUpPieceIndex) {
    SmallString<256> Buf;
    llvm::raw_svector_ostream Out(Buf);
  
    SourceRange Range(Piece.getLocation().asRange());
  
    // Write out the path indices with a right arrow and the message as a row.
    Out << "<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>";
  
  - // If no report made at this range mark the variable and add the end tags.
  + // If no report made at the current range \c Range we could inject the table's end tag as this is the last report on that range. Also this is the first encounter of the range, after that it is safe to insert new entries to the table.
    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.
  +    // Otherwise we are working with multiple notes at the same range, so inject a new row to the table at the end of the range which is the beginning of the table. With that we fill the table "upwards" which is the same order as we emit reports.
      html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", Buf.c_str(),
                           /*IsTokenRange=*/true);
    }
  }



---

Please rename the `variable-popups.c` to `variable-popups-simple.c`, `variable-popups-2.c` to `variable-popups-macro.c`, and here `variable-popups-multiple.c` comes:

  // 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;
    for (unsigned i = 0; i < 3; ++i)
      if (i)
        bar(a); // expected-warning{{1st function call argument is an uninitialized value}}
  }
  
  // CHECK:      <span class='variable'>i
  // CHECK-SAME:   <table class='variable_popup'><tbody><tr>
  // CHECK-SAME:     <td valign='top'>
  // CHECK-SAME:       <div class='PathIndex PathIndexPopUp'>2.1</div>
  // CHECK-SAME:     </td>
  // CHECK-SAME:     <td>'i' is 0</td>
  // CHECK-SAME:   </tr>
  // CHECK-SAME:   <tr>
  // CHECK-SAME:     <td valign='top'>
  // CHECK-SAME:       <div class='PathIndex PathIndexPopUp'>4.1</div>
  // CHECK-SAME:     </td>
  // CHECK-SAME:     <td>'i' is 1</td>
  // CHECK-SAME:   </tr></tbody></table>
  // CHECK-SAME: </span>


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73993/new/

https://reviews.llvm.org/D73993