[PATCH] D30380: Teach lit to expand glob expressions

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/utils/lit/lit/TestRunner.py:750
+            expanded = " ".join(result)
+            ln = ln.replace(m.group(), expanded)
+        ln = doDirectSubstitutions(ln)
----------------
zturner wrote:
> 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.
Right but `re.findall` or `re.finditer` would allow to iterate over the expression with a single regex query.
I'm not worried about efficiency, just readability, the iterative algorithm you're using makes me think that there is a "feature" to the iterative mode (like one expansion depends on the previous one), which I don't think is intentional here (having a file named `%[[.*]]` is what I meant with "infinite loop" earlier.



https://reviews.llvm.org/D30380