[llvm] [StandardInstrumentations]Add support for numeric label (PR #148844)

Jamie Schmeiser via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 3 07:33:04 PDT 2025


================
@@ -1950,10 +1958,9 @@ std::string DotCfgDiffNode::getBodyContent() const {
   // Drop leading newline, if present.
   if (BS.front() == '\n')
     BS1 = BS1.drop_front(1);
-  // Get label.
-  StringRef Label = BS1.take_until([](char C) { return C == ':'; });
   // drop predecessors as they can be big and are redundant
-  BS1 = BS1.drop_until([](char C) { return C == '\n'; }).drop_front();
+  if (BS1.str().find(Label) != std::string::npos)
----------------
jamieschmeiser wrote:

1.  +1
2. this will only remove the first nl, if present, not all of them.
 IIRC (and also based on the comment), the line
`BS1 = BS1.drop_until([](char C) { return C == '\n'; }).drop_front();` was intended to remove the line listing the predecessor blocks.  Consider a block that has many predecessors (eg 10 predecessors).  The graph shows 10 arrows in so you can easily determine all predecessors.  However, now you are going to get a comment listing the 10 predecessors, making this particular node graphically large, making it difficult to see the other nodes in the graph.  Plus the comment is redundant since you can graphically see the predecessors or you would be able to if the comment weren't there.  Therefore, I removed it.  This line would drop the comment by dropping everything until a nl is reached.
If there were no label, I suspect that there might also not be a comment with the predecessors, resulting in the line erasing the first line of code, as you state.  That is why I find this test troubling...it is dropping the line if there is a label, which is testing a symptom rather than the intention.  With an added label, is there still the comment with the predecessors?  If not, something else might still be dropped.
Would it work if you instead tested for a comment since the point of the line is to remove the comment with predecessors?  I think this would be clearer.
3. Okay...Here is an idea and I don't know if would work or if it is worth the effort...  Since the label is artificial and only seen here (right?), then a different ordering could trigger a spurious reporting of a change (since the label is different).  Would it work if you used a negative integer for this initial label (with a comment stating it is artificial) and then ignored a difference of a negative initial label when checking if a change occurred?  If I understand correctly, you would only need to check the initial label for this special case so it shouldn't be too expensive. Of course, if this label persists or is visible elsewhere, then this would not work.  I wouldn't do that in this PR however.

https://github.com/llvm/llvm-project/pull/148844


More information about the llvm-commits mailing list