[lit] Improve the ToolSubst class a bit

[zturner]

Summary

In a separate patch, I'm working on re-working clang and lld's lit config's to call a common use_clang and use_lld functions, respectively. Making this work and be generic enough that it also supports debuginfo-test's use case requires a little bit more power in how we specify a tool substitution. For example, lld doesn't prefix its tool substitutions with %, but clang does. And debuginfo-tests shouldn't fail if lld can't be found, but obviously lld should.

This patch adds the necessary bits to make this possible.

It also renames ToolFilter to ToolSubst, and moves it to its own file, to avoid a cyclic dependency between __init__.py and the other files in the same module.

[zturner]

There are some issues with the introducer concept here, so I'm going to rework this a little.

[zturner]

This got a little bit more complicated. There were a lot of latent issues and inconsistencies in the way we added tool substitutions, but I think these are all gone now and everything is consistent.

The main change here is the improvements to the ToolSubst class to customize various parts of the substitution algorithm. The idea being twofold:

1. Take this logic away from the person providing the name of the tool, so that everybody doesn't do it a little bit differently.
2. Provide a way to specify more complex subsitution handling behavior, such as what happens when a tool can't be found, or how to find a tool (Sometimes people would look in the os PATH, sometimes in a specific set of directories, sometimes in the configuration's PATH.

For an example of #1, there was the ability to specify before and after characters that should fail a match, so that for example if a substitution is llc, then you won't match /llc when it is used as part of a path, or llc.exe when it used as a filename. But not all tools followed these guidelines, and it was done (unnecessarily) on a case-by-case basis without people understanding when/why to do it. And it didn't even handle everything, like a \ in front of a path (it always assume / paths). Now, you have to go out of your way to avoid the default behavior, which is almost always correct.

For #2, the motivation for this is that I want to provide a standardized set of substitutions for a tool. So that for example, debuginfo-tests can say use_clang() and lld can say use_clang() and get the same substitutions that lld and clang themselves use. But if lld fails to find its tools, then that's a fatal error, whereas if debuginfo-tests can't find lld tools, that's expected in certain scenarios. So the ability to customize this is needed.

As an aside, this patch exposed a bug in an existing test where clang-cc1 was being used instead of %clang_cc1, so that is fixed here.

[rnk]

This CL contains a lot of whitespace, formatting, and style changes. Can you commit those separately?

clang/test/lit.cfg.py

230–231 ↗

(On Diff #117887)

These new lines seem unnecessary, the comment is associated with this def.

llvm/test/lit.cfg.py

63–64 ↗

(On Diff #117887)

ditto

llvm/utils/lit/lit/util.py

1 ↗

(On Diff #117887)

This all looks like style/formatting changes. Can you commit that separately?

[zturner]

Removed formatting changes. Technically, I did the formatting changes first (using pyformat), and then rebased the original patch on top of the formatting changes and ran pyformat again.

[rnk]

lgtm

llvm/test/lit.cfg.py

137 ↗

(On Diff #117907)

I don't think we need this FIXME, it's explained above where we compute lli_args.

[zturner]

llvm/test/lit.cfg.py

137 ↗

(On Diff #117907)

It's not explained why some tests rely on the default args and some tests rely on a different set of args. Is that even correct? Reading the comment makes it sound like lli should be used for everything. Most people are probably just going to write lli without thinking about it, because that's how most other tool substitutions work, but it's nto clear from the comment if that's always the right thing to do, and the person writing the test may not even know to check. If anything, the "fix" suggested by the FIXME could be along the lines of "rename %lli to something that more clearly differentiates it from lli.

cfe/trunk/test/Index/recover-bad-code-rdar_7487294.c

-// RUN: not %clang-cc1 -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only %s 2>&1 | FileCheck %s
 
 // IMPORTANT: This test case intentionally DOES NOT use --disable-free.  It
 // tests that we are properly reclaiming the ASTs and we do not have a double free.
 // Previously we tried to free the size expression of the VLA twice.
 
 int foo(int x) {
   int y[x * 3];
   help
 };
 
 // CHECK: 9:3: error: use of undeclared identifier 'help'
 // CHECK:  help

cfe/trunk/test/lit.cfg.py

 # -*- Python -*-
 
 import os
 import platform
 import re
 import subprocess
 import tempfile
 
 import lit.formats
 import lit.util
 
 from lit.llvm import llvm_config
-from lit.llvm import ToolFilter
+from lit.llvm.subst import ToolSubst
+from lit.llvm.subst import FindTool
 
 # Configuration file for the 'lit' test runner.
 
 # name: The name of this test suite.
 config.name = 'Clang'
 
 # testFormat: The test format to use to interpret tests.
 #

 
 llvm_config.with_environment('LD_LIBRARY_PATH', [
                              config.llvm_shlib_dir, config.llvm_libs_dir], append_path=True)
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
     ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
 
+llvm_config.use_default_substitutions()
+
 # Discover the 'clang' and 'clangcc' to use.
 
 
 def inferClang(PATH):
     # Determine which clang to use.
     clang = os.getenv('CLANG')
 
     # If the user set clang in the environment, definitely use that and don't

 config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
 config.substitutions.append(('%pluginext', config.llvm_plugin_ext))
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 
 if config.clang_examples:
     config.available_features.add('examples')
 
 builtin_include_dir = llvm_config.get_clang_builtin_include_dir(config.clang)
-config.substitutions.append(('%clang_analyze_cc1',
-                             '%clang_cc1 -analyze %analyze'))
-config.substitutions.append(('%clang_cc1',
-                             '%s -cc1 -internal-isystem %s -nostdsysteminc'
-                             % (config.clang, builtin_include_dir)))
-config.substitutions.append(('%clang_cpp', ' ' + config.clang +
-                             ' --driver-mode=cpp '))
-config.substitutions.append(('%clang_cl', ' ' + config.clang +
-                             ' --driver-mode=cl '))
-config.substitutions.append(('%clangxx', ' ' + config.clang +
-                             ' --driver-mode=g++ '))
-
-clang_func_map = lit.util.which(
-    'clang-func-mapping', config.environment['PATH'])
-if clang_func_map:
-    config.substitutions.append(
-        ('%clang_func_map', ' ' + clang_func_map + ' '))
 
-config.substitutions.append(('%clang', ' ' + config.clang + ' '))
-config.substitutions.append(('%test_debuginfo',
-                             ' ' + config.llvm_src_root + '/utils/test_debuginfo.pl '))
-config.substitutions.append(('%itanium_abi_triple',
-                             llvm_config.make_itanium_abi_triple(config.target_triple)))
-config.substitutions.append(('%ms_abi_triple',
-                             llvm_config.make_msabi_triple(config.target_triple)))
-config.substitutions.append(('%resource_dir', builtin_include_dir))
-config.substitutions.append(('%python', config.python_executable))
+tools = [
+    # By specifying %clang_cc1 as part of the substitution, this substitution
+    # relies on repeated substitution, so must come before %clang_cc1.
+    ToolSubst('%clang_analyze_cc1', command='%clang_cc1',
+              extra_args=['-analyze', '%analyze']),
+    ToolSubst('%clang_cc1', command=config.clang, extra_args=[
+              '-cc1', '-internal-isystem', builtin_include_dir, '-nostdsysteminc']),
+    ToolSubst('%clang_cpp', command=config.clang,
+              extra_args=['--driver-mode=cpp']),
+    ToolSubst('%clang_cl', command=config.clang,
+              extra_args=['--driver-mode=cl']),
+    ToolSubst('%clangxx', command=config.clang,
+              extra_args=['--driver-mode=g++']),
+    ToolSubst('%clang_func_map', command=FindTool(
+        'clang-func-mapping'), unresolved='ignore'),
+    ToolSubst('%clang', command=config.clang),
+    ToolSubst('%test_debuginfo', command=os.path.join(
+        config.llvm_src_root, 'utils', 'test_debuginfo.pl')),
+    'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 'opt']
 
-# The host triple might not be set, at least if we're compiling clang from
-# an already installed llvm.
-if config.host_triple and config.host_triple != '@LLVM_HOST_TRIPLE@':
-    config.substitutions.append(('%target_itanium_abi_host_triple',
-                                 '--target=%s' % llvm_config.make_itanium_abi_triple(config.host_triple)))
-else:
-    config.substitutions.append(('%target_itanium_abi_host_triple', ''))
+if config.clang_examples:
+    tools.append('clang-interpreter')
 
-config.substitutions.append(
-    ('%src_include_dir', config.clang_src_dir + '/include'))
+# For each occurrence of a clang tool name, replace it with the full path to
+# the build directory holding that tool.  We explicitly specify the directories
+# to search to ensure that we get the tools just built and not some random
+# tools that might happen to be in the user's PATH.
+tool_dirs = [config.clang_tools_dir, config.llvm_tools_dir]
+
+llvm_config.add_tool_substitutions(tools, tool_dirs)
 
 # FIXME: Find nicer way to prohibit this.
 config.substitutions.append(
     (' clang ', """*** Do not use 'clang' in tests, use '%clang'. ***"""))
 config.substitutions.append(
     (' clang\+\+ ', """*** Do not use 'clang++' in tests, use '%clangxx'. ***"""))
 config.substitutions.append(
     (' clang-cc ',
config.substitutions.append(
      """*** invalid substitution, use '%clang_cc1'. ***"""))
 config.substitutions.append(
     (' %clang-cpp ',
      """*** invalid substitution, use '%clang_cpp'. ***"""))
 config.substitutions.append(
     (' %clang-cl ',
      """*** invalid substitution, use '%clang_cl'. ***"""))
 
-# For each occurrence of a clang tool name, replace it with the full path to
-# the build directory holding that tool.  We explicitly specify the directories
-# to search to ensure that we get the tools just built and not some random
-# tools that might happen to be in the user's PATH.
-tool_dirs = [config.clang_tools_dir, config.llvm_tools_dir]
-
-tool_patterns = [
-    'FileCheck', 'c-index-test',
-    ToolFilter('clang-check', pre='-.', post='-.'),
-    ToolFilter('clang-diff', pre='-.', post='-.'),
-    ToolFilter('clang-format', pre='-.', post='-.'),
-    # FIXME: Some clang test uses opt?
-    ToolFilter('opt', pre='-.', post=r'/\-.'),
-    # Handle these specially as they are strings searched for during testing.
-    ToolFilter(r'\| \bcount\b', verbatim=True),
-    ToolFilter(r'\| \bnot\b', verbatim=True)]
+config.substitutions.append(('%itanium_abi_triple',
+                             llvm_config.make_itanium_abi_triple(config.target_triple)))
+config.substitutions.append(('%ms_abi_triple',
+                             llvm_config.make_msabi_triple(config.target_triple)))
+config.substitutions.append(('%resource_dir', builtin_include_dir))
 
-if config.clang_examples:
-    tool_patterns.append(ToolFilter('clang-interpreter', '-.', '-.'))
+# The host triple might not be set, at least if we're compiling clang from
+# an already installed llvm.
+if config.host_triple and config.host_triple != '@LLVM_HOST_TRIPLE@':
+    config.substitutions.append(('%target_itanium_abi_host_triple',
+                                 '--target=%s' % llvm_config.make_itanium_abi_triple(config.host_triple)))
+else:
+    config.substitutions.append(('%target_itanium_abi_host_triple', ''))
 
-llvm_config.add_tool_substitutions(tool_patterns, tool_dirs)
+config.substitutions.append(
+    ('%src_include_dir', config.clang_src_dir + '/include'))
 
 # Set available features we allow tests to conditionalize on.
 #
 if config.clang_default_cxx_stdlib != '':
     config.available_features.add('default-cxx-stdlib-set')
 
 # Enabled/disabled features
 if config.clang_staticanalyzer:
if platform.system() not in ['Windows']:
     config.available_features.add('utf8-capable-terminal')
 
 # Support for libgcc runtime. Used to rule out tests that require
 # clang to run with -rtlib=libgcc.
 if platform.system() not in ['Darwin', 'Fuchsia']:
     config.available_features.add('libgcc')
 
 # Case-insensitive file system
-
-
 def is_filesystem_case_insensitive():
     handle, path = tempfile.mkstemp(
         prefix='case-test', dir=config.test_exec_root)
     isInsensitive = os.path.exists(
         os.path.join(
             os.path.dirname(path),
             os.path.basename(path).upper()
         ))

lld/trunk/test/lit.cfg.py

 # -*- Python -*-
 
 import os
 import platform
 import re
 import subprocess
 import locale
 
 import lit.formats
 import lit.util
 
 from lit.llvm import llvm_config
-from lit.llvm import ToolFilter
+from lit.llvm.subst import ToolSubst
 
 # Configuration file for the 'lit' test runner.
 
 # name: The name of this test suite.
 config.name = 'lld'
 
 # testFormat: The test format to use to interpret tests.
 #

 
 # Tweak the PATH to include the tools dir and the scripts dir.
 llvm_config.with_environment('PATH',
                              [config.llvm_tools_dir, config.lld_tools_dir], append_path=True)
 
 llvm_config.with_environment('LD_LIBRARY_PATH',
                              [config.lld_libs_dir, config.llvm_libs_dir], append_path=True)
 
+llvm_config.use_default_substitutions()
+
 # For each occurrence of a clang tool name, replace it with the full path to
 # the build directory holding that tool.  We explicitly specify the directories
 # to search to ensure that we get the tools just built and not some random
 # tools that might happen to be in the user's PATH.
 tool_dirs = [config.lld_tools_dir, config.llvm_tools_dir]
 
-config.substitutions.append((r"\bld.lld\b", 'ld.lld --full-shutdown'))
-
 tool_patterns = [
-    'FileCheck', 'not', 'ld.lld', 'lld-link', 'llvm-as', 'llvm-mc', 'llvm-nm',
+    ToolSubst('ld.lld', extra_args=['--full-shutdown']),
+    'lld-link', 'llvm-as', 'llvm-mc', 'llvm-nm',
     'llvm-objdump', 'llvm-pdbutil', 'llvm-readobj', 'obj2yaml', 'yaml2obj',
-    ToolFilter('lld', pre='-./', post='-.')]
+    'lld']
 
 llvm_config.add_tool_substitutions(tool_patterns, tool_dirs)
 
-# Add site-specific substitutions.
-config.substitutions.append(('%python', config.python_executable))
-
 # When running under valgrind, we mangle '-vg' onto the end of the triple so we
 # can check it with XFAIL and XTARGET.
 if lit_config.useValgrind:
     config.target_triple += '-vg'
 
 # Running on ELF based *nix
 if platform.system() in ['FreeBSD', 'Linux']:
     config.available_features.add('system-linker-elf')

llvm/trunk/test/lit.cfg.py

 # -*- Python -*-
 
 # Configuration file for the 'lit' test runner.
 
 import os
 import sys
 import re
 import platform
 import subprocess
 
 import lit.util
 import lit.formats
 from lit.llvm import llvm_config
-from lit.llvm import ToolFilter
+from lit.llvm.subst import FindTool
+from lit.llvm.subst import ToolSubst
 
 # name: The name of this test suite.
 config.name = 'LLVM'
 
 # testFormat: The test format to use to interpret tests.
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files. This is overriden
def get_asan_rtlib():
     asan_dylib_dir_pattern = host_lib_dir + \
         '/clang/*/lib/darwin/libclang_rt.asan_osx_dynamic.dylib'
     found_dylibs = glob.glob(asan_dylib_dir_pattern)
     if len(found_dylibs) != 1:
         return ''
     return found_dylibs[0]
 
 
-lli = 'lli'
+llvm_config.use_default_substitutions()
+
+# Add site-specific substitutions.
+config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
+config.substitutions.append(('%shlibext', config.llvm_shlib_ext))
+config.substitutions.append(('%exeext', config.llvm_exe_ext))
+config.substitutions.append(('%host_cc', config.host_cc))
+
+
+lli_args = []
 # The target triple used by default by lli is the process target triple (some
 # triple appropriate for generating code for the current process) but because
 # we don't support COFF in MCJIT well enough for the tests, force ELF format on
 # Windows.  FIXME: the process target triple should be used here, but this is
 # difficult to obtain on Windows.
 if re.search(r'cygwin|mingw32|windows-gnu|windows-msvc|win32', config.host_triple):
-    lli += ' -mtriple=' + config.host_triple + '-elf'
-config.substitutions.append(('%lli', lli))
+    lli_args = ['-mtriple=' + config.host_triple + '-elf']
+
+llc_args = []
 
 # Similarly, have a macro to use llc with DWARF even when the host is win32.
-llc_dwarf = 'llc'
 if re.search(r'win32', config.target_triple):
-    llc_dwarf += ' -mtriple=' + \
-        config.target_triple.replace('-win32', '-mingw32')
-config.substitutions.append(('%llc_dwarf', llc_dwarf))
-
-# Add site-specific substitutions.
-config.substitutions.append(('%gold', config.gold_executable))
-config.substitutions.append(('%go', config.go_executable))
-config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
-config.substitutions.append(('%shlibext', config.llvm_shlib_ext))
-config.substitutions.append(('%exeext', config.llvm_exe_ext))
-config.substitutions.append(('%python', config.python_executable))
-config.substitutions.append(('%host_cc', config.host_cc))
+    llc_args = [' -mtriple=' +
+                config.target_triple.replace('-win32', '-mingw32')]
 
 # Provide the path to asan runtime lib if available. On darwin, this lib needs
 # to be loaded via DYLD_INSERT_LIBRARIES before libLTO.dylib in case the files
 # to be linked contain instrumented sanitizer code.
 ld64_cmd = config.ld64_executable
 asan_rtlib = get_asan_rtlib()
 if asan_rtlib:
     ld64_cmd = 'DYLD_INSERT_LIBRARIES={} {}'.format(asan_rtlib, ld64_cmd)
-config.substitutions.append(('%ld64', ld64_cmd))
 
-# OCaml substitutions.
-# Support tests for both native and bytecode builds.
-config.substitutions.append(('%ocamlc',
-                             '%s ocamlc -cclib -L%s %s' %
-                             (config.ocamlfind_executable, config.llvm_lib_dir, config.ocaml_flags)))
+ocamlc_command = '%s ocamlc -cclib -L%s %s' % (
+    config.ocamlfind_executable, config.llvm_lib_dir, config.ocaml_flags)
+ocamlopt_command = 'true'
 if config.have_ocamlopt:
-    config.substitutions.append(('%ocamlopt',
-                                 '%s ocamlopt -cclib -L%s -cclib -Wl,-rpath,%s %s' %
-                                 (config.ocamlfind_executable, config.llvm_lib_dir, config.llvm_lib_dir, config.ocaml_flags)))
-else:
-    config.substitutions.append(('%ocamlopt', 'true'))
+    ocamlopt_command = '%s ocamlopt -cclib -L%s -cclib -Wl,-rpath,%s %s' % (
+        config.ocamlfind_executable, config.llvm_lib_dir, config.llvm_lib_dir, config.ocaml_flags)
 
-# For each occurrence of an llvm tool name as its own word, replace it
-# with the full path to the build directory holding that tool.  This
-# ensures that we are testing the tools just built and not some random
-# tools that might happen to be in the user's PATH.  Thus this list
-# includes every tool placed in $(LLVM_OBJ_ROOT)/$(BuildMode)/bin
-# (llvm_tools_dir in lit parlance).
-
-# Avoid matching RUN line fragments that are actually part of
-# path names or options or whatever.
-# The regex is a pre-assertion to avoid matching a preceding
-# dot, hyphen, carat, or slash (.foo, -foo, etc.).  Some patterns
-# also have a post-assertion to not match a trailing hyphen (foo-).
-JUNKCHARS = r".-^/<"
 
-required_tools = [
-    'lli', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-config', 'llvm-cov',
+tools = [
+    ToolSubst('%lli', FindTool('lli'), post='.', extra_args=lli_args),
+    ToolSubst('%llc_dwarf', FindTool('llc'), extra_args=llc_args),
+    ToolSubst('%go', config.go_executable, unresolved='ignore'),
+    ToolSubst('%gold', config.gold_executable, unresolved='ignore'),
+    ToolSubst('%ld64', ld64_cmd, unresolved='ignore'),
+    ToolSubst('%ocamlc', ocamlc_command, unresolved='ignore'),
+    ToolSubst('%ocamlopt', ocamlopt_command, unresolved='ignore'),
+]
+
+# FIXME: Why do we have both `lli` and `%lli` that do slightly different things?
+tools.extend([
+    'lli', 'lli-child-target', 'llvm-ar', 'llvm-as', 'llvm-bcanalyzer', 'llvm-config', 'llvm-cov',
     'llvm-cxxdump', 'llvm-cvtres', 'llvm-diff', 'llvm-dis', 'llvm-dsymutil',
     'llvm-dwarfdump', 'llvm-extract', 'llvm-isel-fuzzer', 'llvm-lib',
     'llvm-link', 'llvm-lto', 'llvm-lto2', 'llvm-mc', 'llvm-mcmarkup',
     'llvm-modextract', 'llvm-nm', 'llvm-objcopy', 'llvm-objdump',
     'llvm-pdbutil', 'llvm-profdata', 'llvm-ranlib', 'llvm-readobj',
     'llvm-rtdyld', 'llvm-size', 'llvm-split', 'llvm-strings', 'llvm-tblgen',
     'llvm-c-test', 'llvm-cxxfilt', 'llvm-xray', 'yaml2obj', 'obj2yaml',
-    'FileCheck', 'yaml-bench', 'verify-uselistorder',
-    ToolFilter('bugpoint', post='-'),
-    ToolFilter('llc', pre=JUNKCHARS),
-    ToolFilter('llvm-symbolizer', pre=JUNKCHARS),
-    ToolFilter('opt', JUNKCHARS),
-    ToolFilter('sancov', pre=JUNKCHARS),
-    ToolFilter('sanstats', pre=JUNKCHARS),
-    # Handle these specially as they are strings searched for during testing.
-    ToolFilter(r'\| \bcount\b', verbatim=True),
-    ToolFilter(r'\| \bnot\b', verbatim=True)]
-
-llvm_config.add_tool_substitutions(required_tools, config.llvm_tools_dir)
-
-# For tools that are optional depending on the config, we won't warn
-# if they're missing.
-
-optional_tools = [
-    'llvm-go', 'llvm-mt', 'Kaleidoscope-Ch3', 'Kaleidoscope-Ch4',
-    'Kaleidoscope-Ch5', 'Kaleidoscope-Ch6', 'Kaleidoscope-Ch7',
-    'Kaleidoscope-Ch8']
-llvm_config.add_tool_substitutions(optional_tools, config.llvm_tools_dir,
-                                   warn_missing=False)
+    'yaml-bench', 'verify-uselistorder',
+    'bugpoint', 'llc', 'llvm-symbolizer', 'opt', 'sancov', 'sanstats'])
+
+# The following tools are optional
+tools.extend([
+    ToolSubst('llvm-go', unresolved='ignore'),
+    ToolSubst('llvm-mt', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch3', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch4', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch5', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch6', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch7', unresolved='ignore'),
+    ToolSubst('Kaleidoscope-Ch8', unresolved='ignore')])
+
+llvm_config.add_tool_substitutions(tools, config.llvm_tools_dir)
 
 # Targets
 
 config.targets = frozenset(config.targets_to_build.split())
 
 for arch in config.targets_to_build.split():
     config.available_features.add(arch.lower() + '-registered-target')
 

llvm/trunk/utils/lit/lit/llvm/__init__.py

 from lit.llvm import config
-import lit.util
-import re
 
 llvm_config = None
 
-class ToolFilter(object):
-    """
-        String-like class used to build regex substitution patterns for
-        llvm tools.  Handles things like adding word-boundary patterns,
-        and filtering characters from the beginning an end of a tool name
-    """
-
-    def __init__(self, name, pre=None, post=None, verbatim=False):
-        """
-            Construct a ToolFilter.
-
-            name: the literal name of the substitution to look for.
-
-            pre: If specified, the substitution will not find matches where
-            the character immediately preceding the word-boundary that begins
-            `name` is any of the characters in the string `pre`.
-
-            post: If specified, the substitution will not find matches where
-            the character immediately after the word-boundary that ends `name`
-            is any of the characters specified in the string `post`.
-
-            verbatim: If True, `name` is an exact regex that is passed to the
-            underlying substitution
-        """
-        if verbatim:
-            self.regex = name
-            return
-
-        def not_in(chars, where=''):
-            if not chars:
-                return ''
-            pattern_str = '|'.join(re.escape(x) for x in chars)
-            return r'(?{}!({}))'.format(where, pattern_str)
-
-        self.regex = not_in(pre, '<') + r'\b' + name + r'\b' + not_in(post)
-
-    def __str__(self):
-        return self.regex
-
 
 def initialize(lit_config, test_config):
     global llvm_config
 
     llvm_config = config.LLVMConfig(lit_config, test_config)
-

llvm/trunk/utils/lit/lit/llvm/config.py

 import os
 import platform
 import re
 import subprocess
 import sys
 
 import lit.util
+from lit.llvm.subst import FindTool
+from lit.llvm.subst import ToolSubst
 
 
 def binary_feature(on, feature, off_prefix):
     return feature if on else off_prefix + feature
 
 
 class LLVMConfig(object):
 
def make_msabi_triple(self, triple):
             # If the OS is win32, we're done.
             return triple
         if isa.startswith('x86') or isa == 'amd64' or re.match(r'i\d86', isa):
             # For x86 ISAs, adjust the OS.
             return isa + '-' + vendor + '-win32'
         # -win32 is not supported for non-x86 targets; use a default.
         return 'i686-pc-win32'
 
-    def add_tool_substitutions(self, tools, search_dirs, warn_missing=True):
+    def add_tool_substitutions(self, tools, search_dirs=None):
+        if not search_dirs:
+            search_dirs = [self.config.llvm_tools_dir]
+
         if lit.util.is_string(search_dirs):
             search_dirs = [search_dirs]
 
+        tools = [x if isinstance(x, ToolSubst) else ToolSubst(x)
+                 for x in tools]
+
         search_dirs = os.pathsep.join(search_dirs)
+        substitutions = []
+
         for tool in tools:
-            # Extract the tool name from the pattern.  This relies on the tool
-            # name being surrounded by \b word match operators.  If the
-            # pattern starts with "| ", include it in the string to be
-            # substituted.
-            if lit.util.is_string(tool):
-                tool = lit.util.make_word_regex(tool)
-            else:
-                tool = str(tool)
+            match = tool.resolve(self, search_dirs)
 
-            tool_match = re.match(r"^(\\)?((\| )?)\W+b([0-9A-Za-z-_\.]+)\\b\W*$",
-                                  tool)
-            if not tool_match:
+            # Either no match occurred, or there was an unresolved match that
+            # is ignored.
+            if not match:
                 continue
 
-            tool_pipe = tool_match.group(2)
-            tool_name = tool_match.group(4)
-            tool_path = lit.util.which(tool_name, search_dirs)
-            if not tool_path:
-                if warn_missing:
-                    # Warn, but still provide a substitution.
-                    self.lit_config.note(
-                        'Did not find ' + tool_name + ' in %s' % search_dirs)
-                tool_path = self.config.llvm_tools_dir + '/' + tool_name
-
-            if tool_name == 'llc' and os.environ.get('LLVM_ENABLE_MACHINE_VERIFIER') == '1':
-                tool_path += ' -verify-machineinstrs'
-            if tool_name == 'llvm-go':
-                exe = getattr(self.config, 'go_executable', None)
-                if exe:
-                    tool_path += ' go=' + exe
+            subst_key, tool_pipe, command = match
 
-            self.config.substitutions.append((tool, tool_pipe + tool_path))
+            # An unresolved match occurred that can't be ignored.  Fail without
+            # adding any of the previously-discovered substitutions.
+            if not command:
+                return False
+
+            substitutions.append((subst_key, tool_pipe + command))
+
+        self.config.substitutions.extend(substitutions)
+        return True
+
+    def use_default_substitutions(self):
+        tool_patterns = [
+            ToolSubst('FileCheck', unresolved='fatal'),
+            # Handle these specially as they are strings searched for during testing.
+            ToolSubst(r'\| \bcount\b', command=FindTool(
+                'count'), verbatim=True, unresolved='fatal'),
+            ToolSubst(r'\| \bnot\b', command=FindTool('not'), verbatim=True, unresolved='fatal')]
+
+        self.config.substitutions.append(('%python', sys.executable))
+        self.add_tool_substitutions(
+            tool_patterns, [self.config.llvm_tools_dir])

llvm/trunk/utils/lit/lit/llvm/subst.py

+import os
+import re
+
+import lit.util
+
+expr = re.compile(r"^(\\)?((\| )?)\W+b(\S+)\\b\W*$")
+wordifier = re.compile(r"(\W*)(\b[^\b]+\b)")
+
+
+class FindTool(object):
+    def __init__(self, name):
+        self.name = name
+
+    def resolve(self, config, dirs):
+        command = lit.util.which(self.name, dirs)
+        if not command:
+            return None
+
+        if self.name == 'llc' and os.environ.get('LLVM_ENABLE_MACHINE_VERIFIER') == '1':
+            command += ' -verify-machineinstrs'
+        elif self.name == 'llvm-go':
+            exe = getattr(config.config, 'go_executable', None)
+            if exe:
+                command += ' go=' + exe
+        return command
+
+
+class ToolSubst(object):
+    """String-like class used to build regex substitution patterns for llvm
+    tools.
+
+    Handles things like adding word-boundary patterns, and filtering
+    characters from the beginning an end of a tool name
+
+    """
+
+    def __init__(self, key, command=None, pre=r'.-^/\<', post='-.', verbatim=False,
+                 unresolved='warn', extra_args=None):
+        """Construct a ToolSubst.
+
+        key: The text which is to be substituted.
+
+        command: The command to substitute when the key is matched.  By default,
+        this will treat `key` as a tool name and search for it.  If it is
+        a string, it is intereprted as an exact path.  If it is an instance of
+        FindTool, the specified tool name is searched for on disk.
+
+        pre: If specified, the substitution will not find matches where
+        the character immediately preceding the word-boundary that begins
+        `key` is any of the characters in the string `pre`.
+
+        post: If specified, the substitution will not find matches where
+        the character immediately after the word-boundary that ends `key`
+        is any of the characters specified in the string `post`.
+
+        verbatim: If True, `key` is an exact regex that is passed to the
+        underlying substitution
+
+        unresolved: Action to take if the tool substitution cannot be
+        resolved.  Valid values:
+            'warn' - log a warning but add the substitution anyway.
+            'fatal' - Exit the test suite and log a fatal error.
+            'break' - Don't add any of the substitutions from the current
+                      group, and return a value indicating a failure.
+            'ignore' - Don't add the substitution, and don't log an error
+
+        extra_args: If specified, represents a list of arguments that will be
+        appended to the tool's substitution.
+
+        explicit_path: If specified, the exact path will be used as a substitution.
+        Otherwise, the tool will be searched for as if by calling which(tool)
+
+        """
+        self.unresolved = unresolved
+        self.extra_args = extra_args
+        self.key = key
+        self.command = command if command is not None else FindTool(key)
+        if verbatim:
+            self.regex = key
+            return
+
+        def not_in(chars, where=''):
+            if not chars:
+                return ''
+            pattern_str = '|'.join(re.escape(x) for x in chars)
+            return r'(?{}!({}))'.format(where, pattern_str)
+
+        def wordify(word):
+            match = wordifier.match(word)
+            introducer = match.group(1)
+            word = match.group(2)
+            return introducer + r'\b' + word + r'\b'
+
+        self.regex = not_in(pre, '<') + wordify(key) + not_in(post)
+
+    def resolve(self, config, search_dirs):
+        # Extract the tool name from the pattern.  This relies on the tool
+        # name being surrounded by \b word match operators.  If the
+        # pattern starts with "| ", include it in the string to be
+        # substituted.
+
+        tool_match = expr.match(self.regex)
+        if not tool_match:
+            return None
+
+        tool_pipe = tool_match.group(2)
+        tool_name = tool_match.group(4)
+
+        if isinstance(self.command, FindTool):
+            command_str = self.command.resolve(config, search_dirs)
+        else:
+            command_str = str(self.command)
+
+        if command_str:
+            if self.extra_args:
+                command_str = ' '.join([command_str] + self.extra_args)
+        else:
+            if self.unresolved == 'warn':
+                # Warn, but still provide a substitution.
+                config.lit_config.note(
+                    'Did not find ' + tool_name + ' in %s' % search_dirs)
+                command_str = os.path.join(
+                    config.config.llvm_tools_dir, tool_name)
+            elif self.unresolved == 'fatal':
+                # The function won't even return in this case, this leads to
+                # sys.exit
+                config.lit_config.fatal(
+                    'Did not find ' + tool_name + ' in %s' % search_dirs)
+            elif self.unresolved == 'break':
+                # By returning a valid result with an empty command, the
+                # caller treats this as a failure.
+                pass
+            elif self.unresolved == 'ignore':
+                # By returning None, the caller just assumes there was no
+                # match in the first place.
+                return None
+            else:
+                raise 'Unexpected value for ToolSubst.unresolved'
+
+        return (self.regex, tool_pipe, command_str)

llvm/trunk/utils/lit/lit/util.py

def which(command, paths=None):
     """which(command, [paths]) - Look up the given command in the paths string
     (or the PATH environment variable, if unspecified)."""
 
     if paths is None:
         paths = os.environ.get('PATH', '')
 
     # Check for absolute match first.
     if os.path.isfile(command):
-        return command
+        return os.path.normpath(command)
 
     # Would be nice if Python had a lib function for this.
     if not paths:
         paths = os.defpath
 
     # Get suffixes to search.
     # On Cygwin, 'PATHEXT' may exist but it should not be used.
     if os.pathsep == ';':
         pathext = os.environ.get('PATHEXT', '').split(';')
     else:
         pathext = ['']
 
     # Search the paths...
     for path in paths.split(os.pathsep):
         for ext in pathext:
             p = os.path.join(path, command + ext)
             if os.path.exists(p) and not os.path.isdir(p):
-                return p
+                return os.path.normpath(p)
 
     return None
 
 
 def checkToolsPath(dir, tools):
     for tool in tools:
         if not os.path.exists(os.path.join(dir, tool)):
             return False