Fix update_cc_test_checks.py --llvm-bin after D78478

[arichardson]

Summary

Not passing --clang would result in a python exception after this change:
(TypeError: expected str, bytes or os.PathLike object, not NoneType)
because the --clang argument default was only being populated in the
initial argument parsing pass but not later on.
Fix this by adding an argparse callback to set the default values.

[MaskRay]

clang/test/utils/update_cc_test_checks/lit.local.cfg

24 ↗

(On Diff #280399)

The substitution is here just to make a test. Can it be tested with existing substitutions?

[arichardson]

clang/test/utils/update_cc_test_checks/lit.local.cfg

24 ↗

(On Diff #280399)

I can't think of an easy way since we need to invoke update_cc_test_checks.py without the "--clang" argument and we need a substitution for the path to the update script.

[vitalybuka]

LGTM as it fixes update_cc_test_checks.py

llvm/utils/update_cc_test_checks.py

116–126

I guess empty clang also make no sense, "not args.clang" is better here

[vitalybuka]

clang/test/utils/update_cc_test_checks/lit.local.cfg

24 ↗

(On Diff #280399)

If there are concerns here, I'd recommend to land minimal fix for the tool and move test plumbing into a separate patch.

[MaskRay]

LGTM.

clang/test/utils/update_cc_test_checks/lit.local.cfg

24 ↗

(On Diff #280399)

If there are concerns here, I'd recommend to land minimal fix for the tool and move test plumbing into a separate patch.

+1

[arichardson]

remove tests and check for empty instead of none

llvm/utils/UpdateTestChecks/common.py

def __init__(self, line, line_number, args, argv):
     self.line = line
     self.line_number = line_number
     self.args = args
     self.argv = argv
 
 
 class TestInfo(object):
   def __init__(self, test, parser, script_name, input_lines, args, argv,
-               comment_prefix):
+               comment_prefix, argparse_callback):
     self.parser = parser
+    self.argparse_callback = argparse_callback
     self.path = test
     self.args = args
     self.argv = argv
     self.input_lines = input_lines
     self.run_lines = find_run_lines(test, self.input_lines)
     self.comment_prefix = comment_prefix
     if self.comment_prefix is None:
       if self.path.endswith('.mir'):
         self.comment_prefix = '#'
       else:
         self.comment_prefix = ';'
     self.autogenerated_note_prefix = self.comment_prefix + ' ' + UTC_ADVERT
     self.test_autogenerated_note = self.autogenerated_note_prefix + script_name
     self.test_autogenerated_note += get_autogennote_suffix(parser, self.args)
 
   def iterlines(self, output_lines):
     output_lines.append(self.test_autogenerated_note)
     for line_num, input_line in enumerate(self.input_lines):
       # Discard any previous script advertising.
       if input_line.startswith(self.autogenerated_note_prefix):
         continue
       self.args, self.argv = check_for_command(input_line, self.parser,
-                                               self.args, self.argv)
+                                               self.args, self.argv, self.argparse_callback)
       if not self.args.enabled:
         output_lines.append(input_line)
         continue
       yield InputLineInfo(input_line, line_num, self.args, self.argv)
 
 
-def itertests(test_patterns, parser, script_name, comment_prefix=None):
+def itertests(test_patterns, parser, script_name, comment_prefix=None, argparse_callback=None):
   for pattern in test_patterns:
     # On Windows we must expand the patterns ourselves.
     tests_list = glob.glob(pattern)
     if not tests_list:
       warn("Test file pattern '%s' was not found. Ignoring it." % (pattern,))
       continue
     for test in tests_list:
       with open(test) as f:
         input_lines = [l.rstrip() for l in f]
       args = parser.parse_args()
+      if argparse_callback is not None:
+        argparse_callback(args)
       argv = sys.argv[:]
       first_line = input_lines[0] if input_lines else ""
       if UTC_ADVERT in first_line:
         if script_name not in first_line and not args.force_update:
           warn("Skipping test which wasn't autogenerated by " + script_name, test)
           continue
-        args, argv = check_for_command(first_line, parser, args, argv)
+        args, argv = check_for_command(first_line, parser, args, argv, argparse_callback)
       elif args.update_only:
         assert UTC_ADVERT not in first_line
         warn("Skipping test which isn't autogenerated: " + test)
         continue
       yield TestInfo(test, parser, script_name, input_lines, args, argv,
-                     comment_prefix)
+                     comment_prefix, argparse_callback)
 
 
 def should_add_line_to_output(input_line, prefix_set):
   # Skip any blank comment lines in the IR.
   if input_line.strip() == ';':
     return False
   # Skip any blank lines in the IR.
   #if input_line.strip() == '':
for action in parser._actions:
     autogenerated_note_args += action.option_strings[0] + ' '
     if action.const is None:  # action takes a parameter
       autogenerated_note_args += '%s ' % value
   if autogenerated_note_args:
     autogenerated_note_args = ' %s %s' % (UTC_ARGS_KEY, autogenerated_note_args[:-1])
   return autogenerated_note_args
 
 
-def check_for_command(line, parser, args, argv):
+def check_for_command(line, parser, args, argv, argparse_callback):
     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))
+        if argparse_callback is not None:
+          argparse_callback(args)
     return args, argv

llvm/utils/update_cc_test_checks.py

def get_line2spell_and_mangled(args, clang_args):
   return ret
 
 
 def str_to_commandline(value):
   if not value:
     return []
   return shlex.split(value)
 
+
+def infer_dependent_args(args):
+  if not args.clang:
+    if not args.llvm_bin:
+      args.clang = 'clang'
+    else:
+      args.clang = os.path.join(args.llvm_bin, 'clang')
+  if not args.opt:
+    if not args.llvm_bin:
+      args.opt = 'opt'
+    else:
+      args.opt = os.path.join(args.llvm_bin, 'opt')
+
+
 def config():
   parser = argparse.ArgumentParser(
       description=__doc__,
       formatter_class=argparse.RawTextHelpFormatter)
   parser.add_argument('--llvm-bin', help='llvm $prefix/bin path')
   parser.add_argument('--clang',
                       help='"clang" executable, defaults to $llvm_bin/clang')
   parser.add_argument('--clang-args', default=[], type=str_to_commandline,
                       help='Space-separated extra args to clang, e.g. --clang-args=-v')
   parser.add_argument('--opt',
                       help='"opt" executable, defaults to $llvm_bin/opt')
   parser.add_argument(
       '--functions', nargs='+', help='A list of function name regexes. '
       'If specified, update CHECK lines for functions matching at least one regex')
   parser.add_argument(
       '--x86_extra_scrub', action='store_true',
       help='Use more regex for x86 matching to reduce diffs between various subtargets')
   parser.add_argument('--function-signature', action='store_true',
                       help='Keep function signature information around for the check line')
   parser.add_argument('--check-attributes', action='store_true',
                       help='Check "Function Attributes" for functions')
   parser.add_argument('tests', nargs='+')
   args = common.parse_commandline_args(parser)
+  infer_dependent_args(args)
 
-  if args.clang is None:
-    if args.llvm_bin is None:
-      args.clang = 'clang'
-    else:
-      args.clang = os.path.join(args.llvm_bin, 'clang')
   if not distutils.spawn.find_executable(args.clang):
     print('Please specify --llvm-bin or --clang', file=sys.stderr)
     sys.exit(1)
 
   # Determine the builtin includes directory so that we can update tests that
   # depend on the builtin headers. See get_clang_builtin_include_dir() and
   # use_clang() in llvm/utils/lit/lit/llvm/config.py.
   try:
     builtin_include_dir = subprocess.check_output(
       [args.clang, '-print-file-name=include']).decode().strip()
     SUBST['%clang_cc1'] = ['-cc1', '-internal-isystem', builtin_include_dir,
                            '-nostdsysteminc']
   except subprocess.CalledProcessError:
     common.warn('Could not determine clang builtins directory, some tests '
                 'might not update correctly.')
 
-  if args.opt is None:
-    if args.llvm_bin is None:
-      args.opt = 'opt'
-    else:
-      args.opt = os.path.join(args.llvm_bin, 'opt')
   if not distutils.spawn.find_executable(args.opt):
     # Many uses of this tool will not need an opt binary, because it's only
     # needed for updating a test that runs clang | opt | FileCheck. So we
     # defer this error message until we find that opt is actually needed.
     args.opt = None
 
   return args, parser
 
else:
     sys.exit(1)
 
 
 def main():
   initial_args, parser = config()
   script_name = os.path.basename(__file__)
 
   for ti in common.itertests(initial_args.tests, parser, 'utils/' + script_name,
-                             comment_prefix='//'):
+                             comment_prefix='//', argparse_callback=infer_dependent_args):
     # Build a list of clang command lines and check prefixes from RUN lines.
     run_list = []
     line2spell_and_mangled_list = collections.defaultdict(list)
     for l in ti.run_lines:
       commands = [cmd.strip() for cmd in l.split('|')]
 
       triple_in_cmd = None
       m = common.TRIPLE_ARG_RE.search(commands[0])