[PATCH] D122569: [lit] Support %if ... %else syntax for RUN lines

[Artem Belevich via Phabricator via llvm-commits]

tra added inline comments.


================
Comment at: llvm/docs/TestingGuide.rst:615
 
+``%if feature {<if branch>} %else {<else branch>}``
+
----------------
delcypher wrote:
> asavonic wrote:
> > tra wrote:
> > > Would any of the following be expected to work?
> > > - `%if feature {do_something} | FileCheck %s`   -- this will probably fail as empty substitution would produce no output to check
> > > - `%if feature {do_something | FileCheck %s}` -- this would probably work.
> > > - `%if featureA { %if featureB {do_AB} %else {do_A_notB}} %else {do_notA}` -- I suspect we'll fail to parse it correctly.
> > > 
> > > I'd document that we currently can't nest those `%if/%else` and, maybe add the `%if feature {do_something | FileCheck %s}` as a canonical example of how to use it for conditional output-checking tests.
> > > 
> > > Would any of the following be expected to work?
> > > - `%if feature {do_something} | FileCheck %s`   -- this will probably fail as empty substitution would produce no output to check
> > 
> > Right. If the feature is not available then we'll get ` | FileCheck %s`.
> >  
> > > - `%if feature {do_something | FileCheck %s}` -- this would probably work.
> > 
> > This does not work for verbose output: LIT formats a command into `echo "RUN at line #"; ${command};` and if `${command}` is an empty string we'll get `echo "RUN at line #"; ;`. Bash cannot parse the double semicolon at the end.
> > 
> > We can probably handle this as a special case.
> > 
> > > - `%if featureA { %if featureB {do_AB} %else {do_A_notB}} %else {do_notA}` -- I suspect we'll fail to parse it correctly.
> > 
> > Recursive substitution actually works if `config.recursiveExpansionLimit` is set in lit.cfg. I'll this case to the test.
> > 
> > > 
> > > I'd document that we currently can't nest those `%if/%else` and, maybe add the `%if feature {do_something | FileCheck %s}` as a canonical example of how to use it for conditional output-checking tests.
> > > 
> > 
> > As I mentioned above, a canonical example would be:
> > `%if feature {do_something | FileCheck %s} %else {true}` 
> > 
> I am not a fan of this syntax.
> 
> 1. Being prefixed with `%` suggests it is substitution, but `%if` and `%else` are not normal substitutions. It might be worth introducing different syntax for these.
> 2. Lit has this annoying problem with substitutions are prefixes of other substitutions (e.g. `%s` and `%source`) where the order that substitutions added matter. There's a possibility that by introducing `%if` and `%else` that this will break existing substitutions (e.g. `%if_something` and `%else_something`). You could avoid this by using something like `%if%` and `%else%`.
> LIT formats a command into echo "RUN at line #"; ${command}; and if ${command} is an empty string we'll get echo "RUN at line #"; ;. Bash cannot parse the double semicolon at the end.

Perhaps we should expand the empty branch to `:` which *is* the standard null command in shell https://pubs.opengroup.org/onlinepubs/009695399/utilities/colon.html



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122569