[LIT] Add SuffixDispatchTest for use in libc++ and libc++abi tests.

[EricWF]

Summary

This is an attempt to lift common functionality needed by libc++ and libc++abi into LIT proper.

SuffixDispatchTest handles:

• Reading XFAIL, REQUIRES, UNSUPPORTED, tags from the source file.
• Calculating if a test is unsupported.
• Invoking the test_runner based on the tests suffix.

The tags have the following semantics:

• XFAIL: Tests that are known failures and should be fixed.
• UNSUPPORTED: Tests that are not relevant given a set of features.
• REQUIRES: Mirrors UNSUPPORTED

SuffixDispatchTest allows tests with different suffixes to be handled in different ways. In libc++ .pass.cpp tests must compile and execute successfully and .fail.cpp tests must fail to compile. We can provide two different handlers to SuffixDispatchTest as follows:

test_format = SuffixDispatchTest()
test_format.add_suffix_test('.pass.cpp', PassTestHandler(...))
test_format.add_suffix_test('.fail.cpp', FailTestHandler(...))

Test handlers are expected to provide the following signature:

result,report = handler(test, lit_config)

Where:

• result is a instance of ResultCode
• report is a string representing the test that was run and the output.
• test is an instance of lit.Test
• lit_config is the global lit configuration object.

CompileAndExecuteTestRunner is a test runner for use with SuffixDispatchTest. It attempts to compile and run an executable.

I have patches to move libc++ and libc++abi over to the new format once this is accepted.

[jroelofs]

Excellent, I like it!

You've got a "thumbs up" from me, but I'd like to hear @ddunbar and @danalbert's opinions on it too.

Also, can you post the corresponding libcxxabi and libcxx parts of this in another review?

Cheers,

Jon

utils/lit/lit/formats/suffixdispatchtest.py

39

_parse_metadata_line or _metadata_line_matches perhaps? 'handle' is a bit hand-wavey.

91

Trailing comma.

107

I think you can do this in python....

for (suffix, test) in self.suffix_tests:
  if name.endswith(suffix):
    test_runner = test
    break

112

When does this happen? Should it be an 'assert False'?

utils/lit/lit/util.py

149

Some comments on these two functions would be nice here to explain the "why" behind them. At the moment I don't understand why these are needed.

[EricWF]

@jroelofs Thanks for the review.
I'm going to remove the handling of UNSUPPORTED-XFAIL and REQUIRES-XFAIL tomorrow. They make things needlessly complex and provide little value. Please don't take a look at it until then.

utils/lit/lit/formats/base.py

25–28

This allows suffixes to have multiple extensions ex: .pass.cpp

utils/lit/lit/formats/suffixdispatchtest.py

39

Sounds good thanks.

107

Cool! That's so much cleaner. Thanks.

112

lit_config.fatal will kill the process. This will happen when a test is matched to a suffix in config.suffixes but no test handler has been registered for that suffix.

[jroelofs]

utils/lit/lit/formats/suffixdispatchtest.py

112

Ah. A comment here explaining that might save someone a bit of head-scratching later.

[danalbert]

A bunch of nits, a few design improvements, and a couple questions, but I think this is definitely something that will save us time in the long run. Thanks!

utils/lit/lit/formats/__init__.py

3

I much prefer

from foo import (bar,
                 baz)

Trailing backslashes are gross.

utils/lit/lit/formats/base.py

25

This looks odd to me, but phab isn't giving me any context...

There's an outer loop here somewhere (judging by the yield below). I assume it's looping over each test?

You then have if filepath.endswith(ext) within the for loop, making all but one iteration a no-op. Am I missing something here?

utils/lit/lit/formats/suffixdispatchtest.py

2

I've always loved that Python has an import for a time machine.

5

Sort these.

16

't'.upper()

20

The single worst thing about Python is its syntax for super.

23

Is there ever a case where these aren't known at initialization time of the object? I'd vote for just removing these two methods (especially delete).

85

Alignment.

96

Alignment.

99

Alignment.

107

You wanted a dict.

test_runners = {
    ".pass": PassRunner(),
    ".fail": FailRunner()
}

This block would become test_runner = test_runners[suffix].

114

Spaces after commas

130

Space after comma

148

Why do we have to pass -o for this? Isn't it pretty obvious we want the link command to generate an output?

150

Spaces after commas

exit_code to fit with the rest of the file

152

sac

160

This looks like an implementation detail that leaked (though I appreciate the irony).

I'd like to see if we can factor this out some how... but I'm not coming up with anything right now.

164

Why a docstring?

167

sac

173

sac

utils/lit/lit/util.py

9

sort these

149

Yeah, I find it really difficult to believe that a python library needs its own to_string.

159

Oh, I see it wasn't you that added it. Still confused that we needed it...

189

Why do we need this? What's wrong with subprocess.Popen(..., stdout=subprocess.PIPE, stderr=subprocess.PIPE)?

194

sac (and not before)

206

exit_code

234

Docstrings?

First line added is unconditional, no sense in report = '' before

240

sac

247

sac

276

lit.util doesn't seem like the right place for this (but I haven't had a look around to know what the right place would be).

279

The copy probably isn't necessary.

282

Isn't this implicit?

285

For the way this is used, I think def compilerCmd(self, in, out) would make more sense.

Same goes for the rest of these.

[EricWF]

Remove UNSUPPORTED-XFAIL and REQUIRES-XFAIL tags. I haven't addressed @danalbert's comments yet.

[EricWF]

I've addressed most of @danalbert's comments. Patch to follow.

utils/lit/lit/formats/__init__.py

3

Sounds good. I'll make the change.

utils/lit/lit/formats/base.py

25

Nope, I think you've found a bug. I'll make a change.

utils/lit/lit/formats/suffixdispatchtest.py

16

Nice way to put that :)

23

Perhaps. I need to look into this more but the use case I envisioned was that sub directories may want to add/remove test suffixes.

107

I disagree. The problem is that I don't know how to split the filename into name + suffix. For example a test.fail.cpp. How do you know where to split the test name?

148

To make the interface simple. Then you have to worry about what parameter you pass to compileLinkCmd is the output file and which one is a set of extra arguments.

After playing around with it I decided the simplest interface is to just take an extra list of options.

160

I'm more alright having this here as opposed to in libc++ and libc++abi's lit.cfg. The valgrind stuff is all defined by lit (and is not local to libc++).

164

No idea. Removed.

utils/lit/lit/util.py

149

Not sure. These already existed in the utility library. I'm just trying not to break them :S. I'll inquire as to why they are needed.

189

Color output! By tricking the sub process into thinking it's running in a TTY it will produce color output. I HATE that we currently lose clang pretty formatting when it gets passed through LIT.

206

exitCode is the current naming in the rest of this file.

276

Your probably right but IDK where to put it either. It's a surprising useful abstraction.

279

I disagree. I already have a use-case for it in the patch that makes libc++ use this.

compile_flags = [...]
cxx = Compiler(compile_flags)
compile_flags.extend(sanitizer_args)
sanitized_cxx = Compiler(compile_flags)

282

Don't think so. Python has shallow copy semantics on classes right?

285

I still want to provide extra compile and link flags. meaning that the interface would look like:

def compileCmd(self, in, out=None, compile_flags=[])

Now you have to name the extra flags if you want to pass them. Instead I really thinks it's cleaner to let the user just pass arbitrary flags.
If you feel strongly I'll make the change, but I had originally implemented it as you suggested and I really didn't like it.

[EricWF]

Revision that addresses @danalbert's comments.

[EricWF]

Remove more usage of docstrings.

[danalbert]

utils/lit/lit/formats/suffixdispatchtest.py

23

Hmm. I'm guessing YAGNI. Even if some subdirectory has different needs (sounds unlikely), they can just add it at the top level.

107

What's wrong with testname.split('.')[-2]?

160

Ah, didn't realize it was a common LIT thing. Works for me.

utils/lit/lit/util.py

149

Well, looking at it, it's for handling transformation of a unicode byte-string into a text string... but what I don't understand is why we'd ever have to deal with that input...

189

Oh, yeah, that's rather nice. The implementation is rather disgusting though :) (I blame subprocess.Popen, not you).

What's wrong with -fcolorize=always?

If -fcolorize=always won't work, I think it would be better to make a PtyPipe class and pass that to executeCommand (with the default arugments being subprocess.PIPE).

206

out_pty and err_pty are the other style though. Let's be consistent throughout the file, whatever that may be.

276

We can keep it here for now if no one has a better home for it,

279

This might be the functional programmer in me showing, but I would write that as

compile_flags = [...]
cxx = Compiler(compile_flags)
sanitized_cxx = Compiler(compile_flags + sanitizer_args)

282

I suppose this depends on the resolution of the comments on 279. I think a shallow copy should be sufficient.

285

The above interface looks right to me.

Keep in mind that hard coding the -o at the call site means that we'll have implementation details of the compiler leaking out of the abstraction and each call site will be

if compiler_under_test_is_msvc:
    compiler.compileCmd(whatever_msvc_needs)
else:
    compiler.compileCmd(['-o', out_file, in_file])

The Compiler should be an abstraction. How the compiler (the actual compiler, not Compiler) works should be irrelevant from the outside.

[danalbert]

utils/lit/lit/formats/suffixdispatchtest.py

71

I don't think this TODO should be removed. I read this to mean that REQUIRES and UNSUPPORTED should be moved into lit.test (so they work for any test runner).

125

Alignment.

utils/lit/lit/util.py

95

This docstring format will mess with tools.

"""Short, single sentence (usually), description.

Longer description in
paragraph format.
"""

[danalbert]

Copy in jroelofs' comments from email.

utils/lit/lit/formats/suffixdispatchtest.py

23

jroelofs: What benefit do we get from shoving everything into the constructors?

I'm just not convinced we'll ever have that many of these. All we really get with my approach is not having these two methods, but I'm not convinced that the current approach gives us anything useful.

107

jroelofs: That assumes test suffixes have at most two '.'s in them...

No, it assumes it has at _least_ that many. Given 'foo.bar.baz.buzz.qux.cpp', this will still choose qux, which aiui is the desired behavior (and lets us move to a data structure that actually cleanly represents this data).

utils/lit/lit/util.py

149

jroelofs: C can have unicode in it, no?

I think what I'm actually misunderstanding here is how that then gets handled by python. I'm assuming that python is smart enough to have already done this for anything returned from subprocess.Popen.communicate.

279

jroelofs: Me too.

[jroelofs]

utils/lit/lit/formats/suffixdispatchtest.py

107

What I really meant is that this limits us to test suffixes with at most two '.'s in them. I.e. given foo.bar.baz.buzz.qux.cpp, then this wouldn't work:

test_format.add_suffix_test('.buzz.qux.cpp', FailTestHandler(...))

utils/lit/lit/util.py

159

Uhgreed.

[EricWF]

Addresses most of @danalbert's comments.

[EricWF]

utils/lit/lit/formats/suffixdispatchtest.py

107

Because I wasn't committing to the suffix being the second part of the extension. I much prefer it matching suffixes is the simplest form.

utils/lit/lit/util.py

189

What's wrong with -fcolorize=always?

This solution can be used with any program that by default will output color on a TTY. There was support for this change because FileCheck apparently outputs in color on TTY's as well.

Also GCC doesn't support it, so we would have to detect if the flag is supported.

If -fcolorize=always won't work, I think it would be better to make a PtyPipe class and pass that to executeCommand (with the default >arugments being subprocess.PIPE).

I'll try that out, but I think the semantics are just different enough that it's not likely to work.

206

Ok.

279

This might be the functional programmer in me showing, but I would write that as

compile_flags = [...]
cxx = Compiler(compile_flags)
sanitized_cxx = Compiler(compile_flags + sanitizer_args)

I'll concede that my example is bad style, but copying lists into objects is the existing style in LIT.
Also I don't see how copying it would cause a problem but I do see how not copying it is a problem.

Is there a reason you want the compile_flags in the Compiler object to refer to the same list it was passed?
I don't see the use case for this.

How strongly do you feel about this change?

282

I'll make the change but I suspect this might be needed since you'll likely pass the same Compiler object to multiple test runners. Each of these test runners may want to modify it.

285

Alright, I'll make the change.

[EricWF]

utils/lit/lit/formats/suffixdispatchtest.py

107

It limits us to exactly two. libcxxabi test suit simply uses '.cpp' as the suffix.

[EricWF]

@ddunbar ping. Do you have any thoughts?

[• ddunbar]

For my own context, is part of the motivation here that you want to share
this test format between multiple repositories?

• Daniel

[EricWF]

Ping.

@ddunbar, Do you want me to remove the executeCommandPTY stuff since that is unrelated to the rest of this?

[EricWF]

In D6206#29, @ddunbar wrote:

For my own context, is part of the motivation here that you want to share
this test format between multiple repositories?

• Daniel

Gaa Sorry @ddunbar, I entirely missed this.

Yes. both libc++ and libc++abi will use SuffixDispatchTest. They both currently have different versions of basically the same code.

[• ddunbar]

I see.

Generally speaking, I think it is great to improve the implementation of
this test format, but I don't think that it meets the bar to be included in
lit proper. I want to keep lit pretty general purpose, and this test format
is just so specific to the libc.* use case that its hard to imagine many
other clients.

Part of my hesitation comes from feeling that this isn't such a great test
format to start with. As a format, it isn't all that flexible (mangling
part of the metadata into the filename really doesn't leave much room for
extension, and then if we start putting metadata in the file too we have it
in two places).

Is there another way you can manage sharing the test format implementation
with those repositories?

Alternately, it seems like the main thing going for this test format is it
makes sense when you have a large number of tests which all run the same
action. We actually end up having that problem in other places too (there
are subsets of clang tests which all follow the same pattern), so I wonder
if there might some value in exploring an extension of the ShTest format
that solved this problem explicitly, and then migrating the libc.* tests to
that.

For example, just to brainstorm, if the ShTest format had a way to use a
"prelude" script for all files in a directory, could that be the basis for
using the ShTest format for libc++?

• Daniel

[EricWF]

In D6206#32, @ddunbar wrote:

I see.

Generally speaking, I think it is great to improve the implementation of
this test format, but I don't think that it meets the bar to be included in
lit proper. I want to keep lit pretty general purpose, and this test format
is just so specific to the libc.* use case that its hard to imagine many
other clients.

Part of my hesitation comes from feeling that this isn't such a great test
format to start with. As a format, it isn't all that flexible (mangling
part of the metadata into the filename really doesn't leave much room for
extension, and then if we start putting metadata in the file too we have it
in two places).

Is there another way you can manage sharing the test format implementation
with those repositories?

Alternately, it seems like the main thing going for this test format is it
makes sense when you have a large number of tests which all run the same
action. We actually end up having that problem in other places too (there
are subsets of clang tests which all follow the same pattern), so I wonder
if there might some value in exploring an extension of the ShTest format
that solved this problem explicitly, and then migrating the libc.* tests to
that.

For example, just to brainstorm, if the ShTest format had a way to use a
"prelude" script for all files in a directory, could that be the basis for
using the ShTest format for libc++?

• Daniel

After seeing more of LIT, I agree this is not a good test format. Most of the handling of internal metadata already exists within TestRunner.parseIntegratedTestScript. I'll lift that to where it belongs. I'll also remove Compiler and CompileAndExecuteTestRunner. Those probably shouldn't live inside lit.

After removing all of that there is no more code that really needs to be shared between libc++ and libc++abi. I'll put the executeCommandPty changes in a different patch and submit them separately.

Thanks for the input.

[• ddunbar]

Sounds good!

• Daniel

utils/lit/lit/formats/__init__.py

 from __future__ import absolute_import
 from lit.formats.base import TestFormat, FileBasedTest, OneCommandPerFileTest
+from lit.formats.suffixdispatchtest import (SuffixDispatchTest,
+                                           CompileAndExecuteTestRunner)
 from lit.formats.googletest import GoogleTest
 from lit.formats.shtest import ShTest

utils/lit/lit/formats/base.py

 
             filepath = os.path.join(source_path, filename)
             if not os.path.isdir(filepath):
-                base,ext = os.path.splitext(filename)
-                if ext in localConfig.suffixes:
+                if any((filepath.endswith(ext) for ext in localConfig.suffixes)):
                     yield lit.Test.Test(testSuite, path_in_suite + (filename,),
                                         localConfig)
 

utils/lit/lit/formats/suffixdispatchtest.py

+from __future__ import absolute_import
+import errno
+import os
+import sys
+import tempfile
+
+import lit.Test
+import lit.util
+from .base import FileBasedTest
+
+class SuffixDispatchTest(FileBasedTest):
+    """ A custom test format that stores a list of (suffix, test_runner) pairs.
+
+        SuffixDispatchTest dispatches tests based on their suffix to different
+        test runners. Test runners must have function signature of
+        __call__(self, test, lit_config).
+    """
+    def __init__(self, suffixes=[]):
+        super(FileBasedTest, self).__init__()
+        self.suffix_tests = list(suffixes)
+
+    def execute(self, test, lit_config):
+        while True:
+            try:
+                return self._execute(test, lit_config)
+            except OSError, oe:
+                if oe.errno != errno.ETXTBSY:
+                    raise
+                time.sleep(0.1)
+
+    def _execute(self, test, lit_config):
+        # Extract test metadata from the test file.
+        requires = []
+        unsupported = []
+        # A helper function that reads a line of features that follow a tag.
+        def _parse_metadata_line(ln, tag, out):
+            if tag not in ln:
+                return False
+            items = ln[ln.index(tag) + len(tag):].split(',')
+            items = [s.strip() for s in items if s.strip()]
+            out.extend(items)
+            return True
+        # FIXME: For now, this is cribbed from lit.TestRunner, to avoid
+        # introducing a dependency there. What we more ideally would like to do
+        # is lift the "requires" handling to be a core lit framework feature.
+        with open(test.getSourcePath()) as f:
+            for ln in f:
+                if _parse_metadata_line(ln, 'XFAIL:', test.xfails):
+                    pass
+                elif _parse_metadata_line(ln, 'REQUIRES:', requires):
+                    pass
+                elif _parse_metadata_line(ln, 'UNSUPPORTED:', unsupported):
+                    pass
+                elif not ln.strip().startswith("//") and ln.strip():
+                    # Stop at the first non-empty line that is not a C++
+                    # comment.
+                    break
+
+        missing_required_features = [f for f in requires
+                                     if f not in test.config.available_features
+                                     or f not in test.config.target_triple]
+        if missing_required_features:
+            return (lit.Test.UNSUPPORTED,
+                    "Test requires the following features: %s" % (
+                      ', '.join(missing_required_features)))
+
+        unsupported_features = [f for f in unsupported
+                                if f in test.config.available_features
+                                or f in test.config.target_triple
+                                or f == '*']
+        if unsupported_features:
+            return (lit.Test.UNSUPPORTED,
+                    "Test is unsupported with the following features: %s" % (
+                       ', '.join(unsupported_features)))
+
+        # Dispatch the test based on name and execute it.
+        name = test.path_in_suite[-1]
+        test_runner = None
+        for (suffix, runner) in self.suffix_tests:
+            if name.endswith(suffix):
+                test_runner = runner
+                break
+        if test_runner is None:
+            # Kill lit if we are given a test that doesn't match an existing
+            # suffix
+            lit_config.fatal('Unrecognized test suffix: %s' % name)
+
+        return test_runner(test, lit_config)
+
+##
+
+class CompileAndExecuteTestRunner(object):
+    def __init__(self, cxx, exec_env={}):
+        self.cxx = cxx
+        self.exec_env = dict(exec_env)
+
+    def __call__(self, test, lit_config):
+        source_path = test.getSourcePath()
+        source_dir = os.path.dirname(source_path)
+        exec_file = tempfile.NamedTemporaryFile(suffix="exe", delete=False)
+        exec_path = exec_file.name
+        exec_file.close()
+        report = ""
+
+        try:
+            compile_cmd = self.cxx.compileLinkCmd(source_path, out=exec_path)
+            cmd = compile_cmd
+            out, err, exit_code, report = lit.util.executeCommandAndReport(cmd)
+            if exit_code != 0:
+                return lit.Test.FAIL, report + "Compilation failed unexpectedly!"
+
+            cmd = []
+            if self.exec_env:
+                cmd.append('env')
+                cmd.extend('%s=%s' % (name, value)
+                            for name, value in self.exec_env.items())
+            cmd.append(exec_path)
+            if lit_config.useValgrind:
+                cmd = lit_config.valgrindArgs + cmd
+            out, err, exit_code, report = lit.util.executeCommandAndReport(
+                cmd, cwd=source_dir)
+            report = 'Compiled With: %s\n' % \
+                    ' '.join(["'%s'" % a for a in compile_cmd]) + report
+            if exit_code != 0:
+                return lit.Test.FAIL, report + "Compiled test failed unexpectedly!"
+        finally:
+            try:
+                os.remove(exec_path)
+            except:
+                pass
+        return lit.Test.PASS, report
+##

utils/lit/lit/util.py

 import math
 import os
 import platform
+import pty
+import select
 import signal
 import subprocess
 import sys

 # Close extra file handles on UNIX (on Windows this cannot be done while
 # also redirecting input).
 kUseCloseFDs = not (platform.system() == 'Windows')
+
+def _to_string(str_bytes):
+    if isinstance(str_bytes, str):
+        return str_bytes
+    return str_bytes.encode('utf-8')
+
+def _convert_string(str_bytes):
+    try:
+        return _to_string(str_bytes.decode('utf-8'))
+    except:
+        return str(str_bytes)
+
+
 def executeCommand(command, cwd=None, env=None):
     p = subprocess.Popen(command, cwd=cwd,
                          stdin=subprocess.PIPE,

                          env=env, close_fds=kUseCloseFDs)
     out,err = p.communicate()
     exitCode = p.wait()
-
     # Detect Ctrl-C in subprocess.
     if exitCode == -signal.SIGINT:
         raise KeyboardInterrupt
 
-    def to_string(bytes):
-        if isinstance(bytes, str):
-            return bytes
-        return bytes.encode('utf-8')
-
     # Ensure the resulting output is always of string type.
+    out = _convert_string(out)
+    err = _convert_string(err)
+
+    return out, err, exitCode
+
+def _read_and_close_pty(p):
+    os.close(p[1])
+    with os.fdopen(p[0], 'r') as f:
+        out = ''
+        try:
+            out = _convert_string(f.read())
+        except IOError:
+            pass
+        return out
+
+def executeCommandPTY(command, cwd=None, env=None):
     try:
-        out = to_string(out.decode('utf-8'))
-    except:
-        out = str(out)
-    try:
-        err = to_string(err.decode('utf-8'))
+        out_pty = tuple(pty.openpty())
+        err_pty = tuple(pty.openpty())
+        kwargs = {
+            'stdin': subprocess.PIPE,
+            'stdout': out_pty[1],
+            'stderr': err_pty[1],
+            'cwd': cwd,
+            'env': env,
+            'close_fds':kUseCloseFDs,
+        }
+        p = subprocess.Popen(command, **kwargs)
+        exit_code = None
+        out = ''
+        err = ''
+        while True:
+            exit_code = p.poll()
+            if exit_code is not None:
+                break
+            to_read,_,_ = select.select([out_pty[0], err_pty[0]], [], [], 0.0)
+            for fd in to_read:
+                data = _convert_string(os.read(fd, 4096))
+                if fd == out_pty[0]:
+                    out += data
+                else:
+                    err += data
+        # Detect Ctrl-C in subprocess.
+        if exit_code == -signal.SIGINT:
+            raise KeyboardInterrupt
+        out += _read_and_close_pty(out_pty)
+        err += _read_and_close_pty(err_pty)
     except:
-        err = str(err)
+        for fd in out_pty + err_pty:
+            try:
+                os.close(fd)
+            except:
+                pass
+        raise
+    return out, err, exit_code
+
+
+
+def _executeCommandAndReport(exec_fn, command, cwd=None, env=None):
+    """ Execute a command and generate a report.
+
+    The report will represent the input (command, cwd, and env) as well as
+    the exit code and output of the command.
+    """
+    out, err, exitCode = exec_fn(command, cwd=cwd, env=env)
+    report = "Command: %s\n" % \
+            ' '.join(["'%s'" % a for a in command])
+    if cwd:
+        report += "Current Directory: %s\n" % cwd
+    if env:
+        report += "Environment: %s\n" % \
+            ' '.join(["'%s'='%s'" % (k,env[k]) for k in env])
+    report += "Exit Code: %d\n" % exitCode
+    if out:
+        report += "Standard Output:\n--\n%s--\n" % out
+    if err:
+        report += "Standard Error:\n--\n%s--\n" % err
+    report += '\n'
+    return out, err, exitCode, report
+
+
+def executeCommandAndReport(command, cwd=None, env=None):
+    return _executeCommandAndReport(executeCommand, command, cwd=cwd, env=env)
+
+
+def executeCommandAndReportPTY(command, cwd=None, env=None):
+    return _executeCommandAndReport(executeCommandPTY, command, cwd=cwd, env=env)
 
-    return out, err, exitCode
 
 def usePlatformSdkOnDarwin(config, lit_config):
     # On Darwin, support relocatable SDKs by providing Clang with a

             sdk_path = out
             lit_config.note('using SDKROOT: %r' % sdk_path)
             config.environment['SDKROOT'] = sdk_path
+
+
+# A simple yet very useful abstraction for represeting a compiler.
+# Used in formats/suffixdispatchtest.py
+class Compiler(object):
+    def __init__(self, path, compile_flags=[], link_flags=[]):
+        self.path = str(path)
+        self.compile_flags = list(compile_flags)
+        self.link_flags = list(link_flags)
+
+    @staticmethod
+    def _get_in_out_args(infile, out):
+        """
+            Add the output flags if out is not None.
+            Add the input files. Allow a list of input files to be
+            passed in as well.
+        """
+        args = []
+        if out is not None:
+            args += ['-o', out]
+        if isinstance(infile, list):
+            args += infile
+        else:
+            args += [infile]
+        return args
+
+    def compileCmd(self, infile, out=None, compile_flags=[]):
+        cmd = [self.path, '-c'] + self.compile_flags + compile_flags
+        cmd += Compiler._get_in_out_args(infile, out)
+        return cmd
+
+    def linkCmd(self, infile, out=None, link_flags=[]):
+        cmd = [self.path] + self.link_flags + link_flags
+        cmd += Compiler._get_in_out_args(infile, out)
+        return cmd
+
+    def compileLinkCmd(self, infile, out=None, compile_flags=[], link_flags=[]):
+        cmd = [self.path] + self.compile_flags + compile_flags \
+                           + self.link_flags + link_flags
+        cmd += Compiler._get_in_out_args(infile, out)
+        return cmd
+
+    def compile(self, infile, out=None, compile_flags=[]):
+        return executeCommandAndReport(self.compileCmd(infile, out, compile_flags))
+
+    def link(self, infile, out=None, link_flags=[]):
+        return executeCommandAndReport(self.linkCmd(infile, out, link_flags))
+
+    def compileLink(self, infile, out=None, compile_flags=[], link_flags=[]):
+        return executeCommandAndReport(
+            self.compileLinkCmd(infile, out, compile_flags, link_flags))