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

[Brian Cain via Phabricator via libcxx-commits]

bcain added a comment.

In D61915#1502008 <https://reviews.llvm.org/D61915#1502008>, @ldionne wrote:

> I'd like to better understand what problem this is solving:
>
> - What platform are you testing on?
> - How did you configure CMake?
> - How do you run lit?


Here's what I think are relevant commits that introduced this behavior:

- https://reviews.llvm.org/rCXX356616
- https://reviews.llvm.org/rCXX356640

The problem I get are XPASS on the following tests:

  Unexpected Passing Tests (7):
      libc++ :: std/input.output/file.streams/fstreams/filebuf.members/open_path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/fstream.cons/path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/fstream.members/open_path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/ifstream.cons/path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/ifstream.members/open_path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/ofstream.cons/path.pass.cpp
      libc++ :: std/input.output/file.streams/fstreams/ofstream.members/open_path.pass.cpp

I was able to reproduce this problem with ToT and x86_64 linux if I set LIBCXX_ENABLE_FILESYSTEM to "False" or "OFF" with just `ninja check-cxx`.  AFAICT diffusion/phabricator won't let me upload arbitrary files like the full unabridged logs so I'll find another place to put them.

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?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D61915