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

[Dan Albert]

Copy in jroelofs' comments from email.

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:22
@@ +21,3 @@
+
+    def add_suffix_test(self, suffix, test_runner):
+        self.suffix_tests += [tuple((suffix, test_runner))]
----------------
danalbert wrote:
> EricWF wrote:
> > danalbert wrote:
> > > 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).
> > 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.
> Hmm. I'm guessing YAGNI. Even if some subdirectory has different needs (sounds unlikely), they can just add it at the top level.
> 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.

================
Comment at: utils/lit/lit/formats/suffixdispatchtest.py:106
@@ +105,3 @@
+        test_runner = None
+        for suffix_test in self.suffix_tests:
+            if name.endswith(suffix_test[0]):
----------------
danalbert wrote:
> EricWF wrote:
> > danalbert wrote:
> > > EricWF wrote:
> > > > jroelofs wrote:
> > > > > I think you can do this in python....
> > > > > 
> > > > >   for (suffix, test) in self.suffix_tests:
> > > > >     if name.endswith(suffix):
> > > > >       test_runner = test
> > > > >       break
> > > > Cool! That's so much cleaner. Thanks.
> > > You wanted a dict.
> > > 
> > >     test_runners = {
> > >         ".pass": PassRunner(),
> > >         ".fail": FailRunner()
> > >     }
> > > 
> > > This block would become `test_runner = test_runners[suffix]`.
> > 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?
> What's wrong with `testname.split('.')[-2]`?
> 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).

================
Comment at: utils/lit/lit/util.py:149
@@ +148,3 @@
+
+def _to_string(str_bytes):
+    if isinstance(str_bytes, str):
----------------
danalbert wrote:
> EricWF wrote:
> > danalbert wrote:
> > > jroelofs wrote:
> > > > 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.
> > > Yeah, I find it really difficult to believe that a python library needs its own `to_string`.
> > 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.
> 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...
> 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`.

================
Comment at: utils/lit/lit/util.py:279
@@ +278,3 @@
+        self.path = str(path)
+        self.compile_flags = list(compile_flags)
+        self.link_flags = list(link_flags)
----------------
danalbert wrote:
> EricWF wrote:
> > danalbert wrote:
> > > The copy probably isn't necessary.
> > 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)
> > ```
> 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)
> jroelofs: Me too.

http://reviews.llvm.org/D6206