[PATCH] D38565: [lit] Improve the ToolSubst class a bit

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/test/lit.cfg.py:137
+
+# FIXME: Why do we have both `lli` and `%lli` that do slightly different things?
+tools.extend([
----------------
rnk wrote:
> I don't think we need this FIXME, it's explained above where we compute lli_args.
It's not explained why some tests rely on the default args and some tests rely on a different set of args.  Is that even correct?  Reading the comment makes it sound like `lli` should be used for everything.  Most people are probably just going to write `lli` without thinking about it, because that's how most other tool substitutions work, but it's nto clear from the comment if that's always the right thing to do, and the person writing the test may not even know to check.  If anything, the "fix" suggested by the FIXME could be along the lines of "rename `%lli` to something that more clearly differentiates it from `lli`.


https://reviews.llvm.org/D38565