[PATCH] D85099: [UpdateTestChecks] Match unnamed values like "@[0-9]+" and "![0-9]+"

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/utils/UpdateTestChecks/common.py:316
+# other locations will need adjustment as well.
+IR_VALUE_REGEXP_PREFIX = r'(\s+)'
+IR_VALUE_REGEXP_STRING = r''
----------------
MaskRay wrote:
> Is this a capturing group? If not, use `(?:\s+)
> 
> The 3 in the `range(3, match.lastindex):` below looks strange. I hope we can get rid if it.
It is, we use in 386 for example. I did not change that part.

The 3 is a bit magic, I agree. It basically is to skip this group, the outer group of all "IR values", which I could avoid and the entire match. We have to have some hardcoded values there though, like before. Though I can make it a variable to explain better.


================
Comment at: llvm/utils/UpdateTestChecks/common.py:324
+    IR_VALUE_REGEXP_STRING += IR_VALUE_PREFIXES[i] + r'(' + kind_regexp + r')'
+IR_VALUE_REGEXP_SUFFIX = r'([,\s\(\)]|\Z)'
+IR_VALUE_RE = re.compile(IR_VALUE_REGEXP_PREFIX + r'(' + IR_VALUE_REGEXP_STRING + r')' + IR_VALUE_REGEXP_SUFFIX)
----------------
MaskRay wrote:
> Use non-capturing group
We use this in 386 and other places.


================
Comment at: llvm/utils/UpdateTestChecks/common.py:402
+      while changed:
+          (lines[i], changed) = IR_VALUE_RE.subn(transform_line_vars, lines[i])
   return lines
----------------
MaskRay wrote:
> Do you have an example that not substituting repeatedly can fail?
Line 8 in `llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/various_ir_values.ll.funcsig.expected` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85099