[PATCH] D30380: Teach lit to expand glob expressions

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:727
 
-def applySubstitutions(script, substitutions):
+def applySubstitutions(script, substitutions, tmpDir):
     """Apply substitutions to the script.  Allow full regular expression syntax.
----------------
mehdi_amini wrote:
> Where is tmpDir used?
It's not, I meant to remove this.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:750
+            expanded = " ".join(result)
+            ln = ln.replace(m.group(), expanded)
+        ln = doDirectSubstitutions(ln)
----------------
mehdi_amini wrote:
> I'm not totally sure about why a loop is needed here? Can't it be done in a single pass?
> Also could we write a pattern that would infinite loop this?
> 
> I was expecting first to do `ln = doDirectSubstitutions(ln)` and then process the "glob" expressions.
If you have multiple glob expressions in a single line, each iteration will only find the first one, but we need to replace all of them.

You're right, I can probably replace all the "direct" substitutions first.


https://reviews.llvm.org/D30380