[PATCH] D133943: [Utils] Match function return type and other attributes on the definition in update_cc_test_checks

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

Submitting some comments I didn't notice were queued up.



================
Comment at: llvm/utils/UpdateTestChecks/common.py:540
       attrs = m.group('attrs') if self._check_attributes else ''
+      fundef_attrs_and_ret = m.group('fundef_attrs_and_ret') if self._record_args else ''
       # Determine if we print arguments, the opening brace, or nothing after the
----------------
arichardson wrote:
> How about 'funcdef'? I initially read this as being related to 'undef'.
I've gone ahead and changed all 'fundef' to 'funcdef'


================
Comment at: llvm/utils/update_test_checks.py:179
                                func_name, args.preserve_names, args.function_signature,
-                               global_vars_seen_dict,
+                               False, global_vars_seen_dict,
                                is_filtered=builder.is_filtered())
----------------
arichardson wrote:
> Why is this hardcoded to false, shouldn't it match the cc test checks version and use args.function_signature?
I think the churn converting all update_cc_test_checks files to match the function return type + attributes isn't too bad, but there are quite a lot more files that would be impacted by changing behaviour in update_test_checks.py. I'm not opposed to making that change here too, but wondered if it's worth changing behaviour for just update_cc_test_checks first?


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

https://reviews.llvm.org/D133943