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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to