[libcxx-commits] [libcxx] [libc++] Make libcxx/selftest a top-level test directory (PR #144852)

[Louis Dionne via libcxx-commits]

ldionne wrote:

> > > `test/libcxx/selftest/modules` moved to `test/libcxx/modules`, since the assumptions these tests check are libc++-specific
> > 
> > 
> > This is tricky -- these tests are really testing the Lit testing format in itself. They are testing the `MODULE_DEPENDENCIES:` directive.
> > I believe it makes more sense to keep them under `libcxx/test/selftest/` even though they don't currently work with other implementations. In theory, these tests could also be used to test other implementations since we're all supposed to be providing a similar mechanism for using C++20 modules (which is what the build systems will rely on).
> > WDYT?
> 
> I have a few problems with this: (1) Every other test _does_ actually work with other implementations AFAICT, so we'd be breaking "this directory can be portably tested" for a theoretical "should be possible to do portable" that we haven't achieved. (2) At least some of them definitely test libc++-specific things. E.g `no-modules.sh.cpp` tests that the normal build doesn't use modules. I don't see how this could ever be non-libc++-specific. Implementations are absolutely allowed to use modules as an implementation detail. (3) IMO these tests are inherently implementation-specific, since they test for compiler flags. We can move them to something like "GCC/Clang-style implementation", but I don't think we can generalize further.

After discussion just now, I think we agree that the intent of the test is to check that `MODULES_DEPENDENCIES: <nothing>` doesn't result in modules compile flags being added, which is a property of the Lit test framework and isn't specific to libc++ as a standard library implementation. The way the test is currently implemented is definitely tied to libc++, though. But I think the organization of the test directories should reflect the intent of the tests, not the way they currently happen to be implemented.

We should add

```
// TODO: This test is currently written in a way that is specific to libc++, but it's really trying to test a property
//           of the test framework, which isn't libc++ specific.
// REQUIRES: stdlib=libc++
```

https://github.com/llvm/llvm-project/pull/144852