[libcxx-commits] [PATCH] D110433: [libc++] Add the std::views::common range adaptor

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/test/std/ranges/range.adaptors/range.common.view/adaptor.nodiscard.verify.cpp:12
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+// REQUIRES: libc++
+
----------------
ldionne wrote:
> jloser wrote:
> > Question: since this is a `libc++` extension, should we move this file into `libcxx/test/libcxx` for example?
> Yes, the convention would be to put those in `libcxx/test/libcxx`. However, I've been sort of on the fence about this recently, it feels easier and clearer to put those alongside the normal tests and just use `REQUIRES: libc++` to express that it's a libc++ specific test. The benefits I see are that you don't have to search for tests in two directories, and also there's no risk of the folder hierarchies getting out of sync if they get moved around.
> 
> I'll move it to `libcxx/test/libcxx` if you request it, otherwise I'll leave as-is.
FWIW, I'm OK leaving it where it is now alongside the other tests. I prefer that, but will point out we have some inconsistency it seems. We can clean it up at some point in the future if desired (I'd be in favor of it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110433