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

Vlad Serebrennikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 16 08:55:22 PDT 2023


Endill added a comment.

In D150856#4427405 <https://reviews.llvm.org/D150856#4427405>, @jdenny wrote:

> For-each is one of the major use cases originally targeted by DEFINE/REDEFINE.  In the C++ DR scenario, the data set to iterate (the split-out files) is already harcoded in the lit test file.  Without any new lit features, I believe you can use DEFINE/REDEFINE now to iterate that data set as follows:
>
>   // RUN: split-file %s %t
>   // DEFINE: %{file} =
>   // DEFINE: %{run} = \
>   // DEFINE:     cmd1 %t/%{file} && \
>   // DEFINE:     cmd2 && cmd3 && cmd4 
>   
>   //--- dr100.cpp
>   // REDEFINE: %{file} = dr100.cpp
>   // RUN: %{run}
>   contents of dr100.cpp 
>   
>   //--- dr101.cpp
>   // REDEFINE: %{file} = dr101.cpp
>   // RUN: %{run}
>   contents of dr101.cpp
>
> Especially from the perspective of lit maintenance, that solution seems simpler to me than inventing additional for-each mechanisms, such as %{for-each-file}.

I don't think we're inventing //additional// iteration mechanisms here, because manual loop unrolling is anything but an iteration mechanism.

> Moreover, it seems more flexible.  For example, some split-out files could be visited by a different command list, or some could be left out of the iteration entirely in order to serve some sort of auxiliary role in the test.

When we need such flexibility, we move the test out of drNxx.cpp into separate file. It's been working for us well for quite a while, so we're unlikely to ask lit to support such flexibility on its side.

Anyway, I went ahead and transformed `dr5xx.cpp` according to your ideas: https://gist.github.com/Endilll/d0099df11e532fb2a7ef7ee34f3cbf21/revisions?diff=split
Line count went up from ≈1k to ≈1.5k.
Note that total C++ CWG DR count approaches 3000, so this file is about 3% of full test suite we're going to have at some point.
We're also can't chain compiler invocations via `&&`: `RUN: %{run98} && %{run11} && %{run14} && %{run17} && %{run20} && %{run23}`, because having dedicated stdout log per each language mode makes DR test debugging workflow much better. To be fully honest, it doesn't work today, but chaining compiler invocations would mean to give up on that entirely.

> If anyone is bothered by the repetition of each file name, then perhaps someone should invent a built-in lit substitution that expands to the most recent split-out file name.  Maybe %{split-file-name}. In each RUN line, it should expand after all custom substitutions.  Then you could have:
>
>   // RUN: split-file %s %t
>   // DEFINE: %{run} = \
>   // DEFINE:     cmd1 %t/%{split-file-name} && \
>   // DEFINE:     cmd2 && cmd3 && cmd4 
>   
>   //--- dr100.cpp
>   // RUN: %{run}
>   contents of dr100.cpp 
>   
>   //--- dr101.cpp
>   // RUN: %{run}
>   contents of dr101.cpp

I'm sympathetic to the intent, but I don't think we should introduce interactions between split-file and lit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150856



More information about the llvm-commits mailing list