Skip to content

Conversation

@ruiling
Copy link
Contributor

@ruiling ruiling commented May 15, 2025

We often add inline comment in mir. It is useful to keep them.

We often add inline comment in mir. It is useful to keep them.
@llvmbot
Copy link
Member

llvmbot commented May 15, 2025

@llvm/pr-subscribers-testing-tools

Author: Ruiling, Song (ruiling)

Changes

We often add inline comment in mir. It is useful to keep them.


Full diff: https://github.com/llvm/llvm-project/pull/140016.diff

1 Files Affected:

  • (modified) llvm/utils/update_mir_test_checks.py (+2-2)
diff --git a/llvm/utils/update_mir_test_checks.py b/llvm/utils/update_mir_test_checks.py
index 86147034d946b..8deaec74f2777 100755
--- a/llvm/utils/update_mir_test_checks.py
+++ b/llvm/utils/update_mir_test_checks.py
@@ -339,9 +339,9 @@ def mangle_vreg(opcode, current_names):
 
 
 def should_add_line_to_output(input_line, prefix_set):
-    # Skip any check lines that we're handling as well as comments
+    # Skip any check lines that we're handling as well as blank comment.
     m = common.CHECK_RE.match(input_line)
-    if (m and m.group(1) in prefix_set) or re.search("^[ \t]*;", input_line):
+    if (m and m.group(1) in prefix_set) or input_line.strip() == ";":
         return False
     return True
 

@arichardson
Copy link
Member

Could you add test? Otherwise LGTM

@ruiling
Copy link
Contributor Author

ruiling commented May 19, 2025

Could you add test? Otherwise LGTM

Thanks @arichardson. I just changed one test to catch this.

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@ruiling ruiling merged commit b8e5307 into llvm:main May 20, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants