[PATCH] D65793: [UpdateTestChecks] Update tests option

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 02:53:37 PDT 2019


MaskRay added inline comments.


================
Comment at: utils/UpdateTestChecks/common.py:76
+def warn(msg, test_file=None):
+    if test_file:
+        msg = '{}: {}'.format(test_file, msg)
----------------
This file is indented by 2.


================
Comment at: utils/UpdateTestChecks/common.py:297
+
+def is_autogenerated_test_file(input_lines):
+  return len(input_lines) > 0 and 'autogenerated' in input_lines[0]
----------------
I don't know if a common function is useful, ideally an `update*.py` utility should abort if the test file was generated by another `update*.py` utility, e.g.

`update_llc_test_checks.py` should abort if the file was generated by `update_test_checks.py`.



================
Comment at: utils/update_analyze_test_checks.py:102
+      if '|' not in l:
+        common.warn('Skipping unparseable RUN line: %s' % (l,))
+        continue
----------------
Use `+` to concatenate two strings.


================
Comment at: utils/update_cc_test_checks.py:139
+      if not common.is_autogenerated_test_file(input_lines):
+        common.warn('Skipping test which isn\'t autogenerated: %s' % (filename))
+        continue
----------------
Use `+` to concatenate two strings.


================
Comment at: utils/update_cc_test_checks.py:162
+      if '|' not in l:
+        common.warn('Skipping unparseable RUN line: %s' % (l,))
+        continue
----------------
If it is clear that `l` in `'... %s' % (l,)` is a `str`, the pattern should be converted to

`'... ' + l` for clarity.

`%s` is not printf C string, it is `str()` applied on the expression... (So `'%s' % ([1,2,3],)` works)


================
Comment at: utils/update_llc_test_checks.py:84
+      if '|' not in l:
+        common.warn('Skipping unparseable RUN line: %s' % (l,))
+        continue
----------------
Use `+` to concatenate two strings.


================
Comment at: utils/update_mir_test_checks.py:117
+        if '|' not in l:
+            common.warn('Skipping unparseable RUN line: %s' % (l,))
+            continue
----------------
Use `+` to concatenate two strings.


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

https://reviews.llvm.org/D65793





More information about the llvm-commits mailing list