[PATCH] D150856: [lit] Add %{for-each-file} substitution

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D150856#4402788 <https://reviews.llvm.org/D150856#4402788>, @jhenderson wrote:

> So I definitely support the overall goal of this patch, but I'm not convinced by the syntax, if I'm honest. It seems somewhat unintuitive that the `%{for-each-file ...}` syntax impacts the whole line it appears on, yet its position is important.
>
> I don't have an exact syntax in mind, but it feels to me like a new lit directive might be more appropriate to indicate that the line should be run multiple times in some manner. It would probably need to involve a new substitution too, admittedly, which indicates where to add the file name in the command. Something along the lines of `RUN-ALL-FILES-%t: cat %f`. Sorry, I don't really have the time right now to give a more thought out example that dictates how to specify the file location.

FWIW, I'm not certain I agree with something long the lines of a new `RUN` syntax. The reason I favor the current `%{for-each-file}` approach is that it works well for more complex scenarios and it's extensible. The `RUN` line approach looks really reasonable when the `RUN` line is simple, but DR testing will often have fairly complex RUN lines that I'm not certain would be improved by a `RUN-ALL-FILES` (for example, complex `-verify` prefixes). But more interestingly, the `%{for-each-file}` approach means we can do a cartesian product of options on a RUN line, which is both terrifying and powerful. e.g., `// RUN: %clang_cc1 %{for-each-file %S} -include %{for-each-file Inputs/}` to generate a RUN line that's the cross product of all the files and all the includes. We don't support that today in the current patch, but it's possible to support in the future if needed. (This may or may not even generalize to something more powerful using `DEFINE` and `REDEFINE` syntax to define "sets" of arbitrary command line options (not just files) that get for-eached on a single RUN line. But I'm not suggesting we go that far with the feature right now, by any means.)

That said, I'm sympathetic that it's a bit weird that a random replacement somewhere in the RUN line means the whole RUN line is duplicated. But I think this functionality will be used in a limited enough way that it won't be overly confusing for long (people who work on tests likely to use this function are most often not writing one-off tests but instead working on a bunch of tests in the same area).

WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150856/new/

https://reviews.llvm.org/D150856