[UpdateTestChecks] Make generation of UTC_ARGS: comment more robust

[arichardson]

Summary

We now use the argparse Action objects to determine the name of the flags.
This fixes cases where the key for the stored result ('dest') is not the
same as the command line flag (e.g. --enable/--disable).
Also add a test that --disabled can be part of the initial UTC_ARGS.

This is split out from D78478

[jdoerfert]

LGTM, except that we might need an explicit test, see below.

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected

11

I'm afraid with this line we do not test if the disable in the first line has an effect. We might need another test, sorry.

[arichardson]

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected

11

We do, without the first disable there would be check lines for the first function. (See the other .expected file for this test)

llvm/test/tools/UpdateTestChecks/update_test_checks/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --function-signature --scrub-attributes --disable
+; RUN: opt -S < %s | FileCheck %s
+
+declare void @foo()
+
+define void @check_lines_1() {
+  ret void
+}
+
+; UTC_ARGS: --disable
+
+; A check line that would not be auto generated.
+; CHECK: define void @no_check_lines() {
+define void @no_check_lines() {
+  ret void
+}
+
+; UTC_ARGS: --enable
+
+define void @check_lines_2() {
+; CHECK-LABEL: define {{[^@]+}}@check_lines_2()
+; CHECK-NEXT:    ret void
+;
+  ret void
+}
+
+define void @scrub() {
+; CHECK-LABEL: define {{[^@]+}}@scrub()
+; CHECK-NEXT:    call void @foo()
+; CHECK-NEXT:    ret void
+;
+  call void @foo() readnone
+  ret void
+}
+
+define i32 @signature(i32 %arg) {
+; CHECK-LABEL: define {{[^@]+}}@signature
+; CHECK-SAME: (i32 [[ARG:%.*]])
+; CHECK-NEXT:    ret i32 [[ARG]]
+;
+  ret i32 %arg
+}

llvm/test/tools/UpdateTestChecks/update_test_checks/on_the_fly_arg_change.test

 # RUN: cp -f %S/Inputs/on_the_fly_arg_change.ll %t.ll
 # RUN: %update_test_checks %t.ll --function-signature --scrub-attributes
 # RUN: diff -u %t.ll %S/Inputs/on_the_fly_arg_change.ll.expected
 ## Check that running the script again does not change the result:
 # RUN: %update_test_checks %t.ll
 # RUN: diff -u %t.ll %S/Inputs/on_the_fly_arg_change.ll.expected
+## Check that the --disable flag is added to the initial UTC_ARGS:
+# RUN: cp -f %S/Inputs/on_the_fly_arg_change.ll %t.ll
+# RUN: %update_test_checks --function-signature --scrub-attributes --disable %t.ll
+# RUN: diff -u %t.ll %S/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected
+## Check that the --disable flag from UTC_ARGS is used:
+# RUN: %update_test_checks %t.ll
+# RUN: diff -u %t.ll %S/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected
+## Check that UTC_ARGS: is parsed after the real command line arguments:
+# RUN: %update_test_checks --enable %t.ll
+# RUN: diff -u %t.ll %S/Inputs/on_the_fly_arg_change.ll.initially_disabled.expected

llvm/utils/UpdateTestChecks/common.py

if "check-prefix=" in part:
       check_prefix(prefix)
     elif "check-prefixes=" in part:
       prefixes = part.split('=', 1)[1].split(',')
       for prefix in prefixes:
         check_prefix(prefix)
         if prefixes.count(prefix) > 1:
           warn("Supplied prefix '%s' is not unique in the prefix list." % (prefix,))
 
+
 def get_autogennote_suffix(parser, args):
-    autogenerated_note_args = ''
-    for k, v in args._get_kwargs():
-        if parser.get_default(k) == v or k == 'tests' or k == 'update_only' or k == 'opt_binary':
-            continue
-        k = k.replace('_', '-')
-        if type(v) is bool:
-            autogenerated_note_args += '--%s ' % (k)
-        else:
-            autogenerated_note_args += '--%s %s ' % (k, v)
-    if autogenerated_note_args:
-        autogenerated_note_args = ' %s %s' % (UTC_ARGS_KEY, autogenerated_note_args[:-1])
-    return autogenerated_note_args
+  for action in parser._actions:
+    if not hasattr(args, action.dest):
+      continue  # Ignore options such as --help that aren't included in args
+    # Ignore parameters such as paths to the binary or the list of tests
+    if action.dest in ('tests', 'update_only', 'opt_binary', 'llc_binary',
+                       'clang', 'opt', 'llvm_bin', 'verbose'):
+    value = getattr(args, action.dest)
+    if action.const is not None:  # action stores a constant (usually True/False)
+      # Skip actions with different constant values (this happens with boolean
+      # --foo/--no-foo options)
+      if value != action.const:
+        continue
+    if parser.get_default(action.dest) == value:
+      continue  # Don't add default values
+    autogenerated_note_args += action.option_strings[0] + ' '
+    if action.const is None:  # action takes a parameter
+      autogenerated_note_args += '%s ' % value
 
 
 def check_for_command(line, parser, args, argv):
     cmd_m = UTC_ARGS_CMD.match(line)
     if cmd_m:
         cmd = cmd_m.group('cmd').strip().split(' ')
         argv = argv + cmd
         args = parser.parse_args(filter(lambda arg: arg not in args.tests, argv))
     return args, argv