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

Tue May 30 23:51:53 PDT 2023

jhenderson added a comment.

In D150856#4382988 <https://reviews.llvm.org/D150856#4382988>, @MaskRay wrote:

> Can you add some tests? Check out `git log -- llvm/utils/lit/tests/` for recent changes.
> `D132513` is a recent major feature (`DEFINE` and `REDEFINE`). Reading its discussions may give some clue about how to add tests.

+1 to this. I'm struggling to envision how this will look in the lit file, which is in turn preventing me from properly providing my own feedback on the feature.`

================
Comment at: llvm/utils/lit/lit/TestRunner.py:1532
     def escapePercents(ln):
+        if isinstance(ln, list):
+            return [_caching_re_compile("%%").sub("#_MARKER_#", line)
----------------
If you haven't already, please make sure to run `black` to format your new python code.

================
Comment at: llvm/utils/lit/lit/TestRunner.py:1651-1652
+            raise ValueError(
+                "Multiple %{for-each-file} per directive are not supported"
+                " yet."
+            )
----------------
Nit: here and with the other error messages, it probably makes sense to follow the standard LLVM coding standard for warnings and errors (no leading capital or trailing full stop).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150856