[PATCH] [libcxx] Add compiler utility class for LIT tests

Dan Albert danalbert at google.com
Fri Jan 16 14:12:10 PST 2015


I tried pretty hard to find a way that this wouldn't work under msvc, but it looks like it all should be fine. It won't pass along the compiler information for msvc, but it will still work.

Mostly LGTM, but I left some nits.


================
Comment at: test/libcxx/test/compiler.py:5
@@ +4,3 @@
+
+class CXXCompiler(object):
+    def __init__(self, path, flags=[], compile_flags=[], link_flags=[], use_ccache=False):
----------------
I probably would have put this directly in the `libcxx` module rather than `libcxx.test`, but nbd.

================
Comment at: test/libcxx/test/compiler.py:13
@@ +12,3 @@
+        self.type = None
+        self.version = (None, None, None)
+        self._initTypeAndVersion()
----------------
This should maybe be just `None`, since you want it to abort if try to use it before it's initialized (and `bool((None, None, None)) == True`).

================
Comment at: test/libcxx/test/compiler.py:20
@@ +19,3 @@
+        if macros is None:
+            return
+        compiler_type = None
----------------
Isn't this an error? Should probably raise an exception...

================
Comment at: test/libcxx/test/compiler.py:48
@@ +47,3 @@
+            cmd += infiles
+        elif isinstance(infiles, str):
+            cmd += [infiles]
----------------
I'm not a fan of using one argument to accept multiple types that don't conform to the same interface (or behave like the same duck, as it were). Just require that `infiles` is a `list`. The argument name is plural, after all.

================
Comment at: test/libcxx/test/compiler.py:102
@@ +101,3 @@
+            return None
+        parsed_macros = dict()
+        lines = [l.strip() for l in out.split('\n') if l.strip()]
----------------
`parsed_macros = {}`

================
Comment at: test/libcxx/test/compiler.py:107
@@ +106,3 @@
+            l = l[len('#define '):]
+            macro, _, value = l.partition(' ')
+            parsed_macros[macro] = value
----------------
`macro, value = l.split()`

http://reviews.llvm.org/D7019

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list