[PATCH] D107162: [lit] Have REQUIRES support the target triple

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added inline comments.


================
Comment at: llvm/docs/TestingGuide.rst:472-473
 | ``REQUIRES`` enables the test if all expressions are true.
-| ``UNSUPPORTED`` disables the test if any expression is true.
+| ``UNSUPPORTED`` disables the test if any expression is true, and takes
+precedence over ``REQUIRES``.
 | ``XFAIL`` expects the test to fail if any expression is true.
----------------
probinson wrote:
> jdenny wrote:
> > probinson wrote:
> > > jdenny wrote:
> > > > I'm not sure this comment about precedence is meaningful.  It seems to say that, if you have an UNSUPPORTED directive, any REQUIRES directive is ignored.  Actually, in the implementation, REQUIRES is checked first.
> > > > 
> > > > I think there's no sense of precedence.  The test runs if REQUIRES is matched and UNSUPPORTED isn't matched.  I think they could be evaluated in either order.
> > > They can be evaluated in either order; but if they are both true, which one wins?  The one that has precedence.
> > You could just as easily ask: If they are both false, which one wins?
> Now that I think about it, REQUIRES isn't required :) so by default tests are enabled.  That means REQUIRES doesn't actually enable a test, So really we should say:
> ```
> REQUIRES disables the test if any expression is false.
> UNSUPPORTED disables the test if any expression is true.
> ```
> Then there's no debate about precedence; both clauses act to disable the test.  Which is how it actually works.
Yes, I like that better too.

And now I get what led to the suggestion of precedence.  :-)


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

https://reviews.llvm.org/D107162