[PATCH] D30380: Teach lit to expand glob expressions

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 15:34:10 PST 2017


zturner added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:147
+    result = [args[0]]
+    expr = re.compile(r"%\[\[(.*?)]]")
+    for arg in args[1:]:
----------------
rnk wrote:
> Why look for double square brackets at all? The lit shell is supposed to be bash compatible, since we use bash on Linux and the internal shell on Windows. Can we just look for '*'?
`*` is not the only character that might indicate the presence of a glob.  There's also `?`.  And character classes in the form of `[abc]` are also valid glob patterns.  We could look for the presence of either of these two characters, but the idea behind the double square brackets was to force the user to acknowledge that they're doing this, as well as to maintain consistency with the existing substitution patterns.  So I do believe we need some kind of delimeter that says "Everything inside of here is a glob expression", and from there we just need to decide what is the best choice that allows the full syntax of a glob expression inside, while not interfering with valid filenames.

Now that I've pointed out `[abc]` though, this raises a concern that `%[[Foo[abc]]]` would be incorrectly parsed as `Foo[abc`.  I used square braces originally so as not to confuse with `%{pathsep}`, but since we've already agreed that curly braces are ok and will not interfere with the shell, how about `%{{expr}}`?


https://reviews.llvm.org/D30380





More information about the llvm-commits mailing list