[libcxx-commits] [PATCH] D61915: [libcxx] change dylib- XFAILs to UNSUPPORTED

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added a comment.

In D61915#1502132 <https://reviews.llvm.org/D61915#1502132>, @bcain wrote:

> AFAIK the idiom for XFAIL is intended for a WIP feature or a defect that can't or shouldn't be fixed (yet) and if this test starts passing it requires some investigation.  I am assuming that dylib-has-no-* is just indicating that a relevant feature for this test is missing or disabled on this target, and as such I would suggest that it doesn't make sense to execute the test at all if the feature is missing.  And if it passes for some reason in this configuration it's not interesting and worth investigation.  Maybe the test no longer depends on the feature, but is that worthy of failing the test suite?  So that's why I have applied this change to all similar tests, not just the ones that I see the XPASS on.
>
> But maybe that's not the case?  Maybe these tests don't really depend on this feature?  Or maybe r356640 should have created a new feature instead of wiring ENABLE_FILESYSTEM into the dylib-has-no-filesystem one?


That's not my thinking.

- XFAIL is "expected to fail"  (for whatever reason)
- UNSUPPORTED is "this test is not worth running"

(I'm going to comment on just the `variant`/`optional`/ `any` tests here, because I think they're straightforward).
They're marked as XFAIL when the dylib doesn't have the appropriate exception classes - and they should fail.
They also should fail if they are built with exceptions disabled - because the exceptions will never get thrown. This behavior should be confirmed.

If they pass, then something very strange is going on, and should be investigated. 
Either the test is not testing what we want, or the dylib is not getting built in the manner that we expect.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61915