[libcxx-commits] [PATCH] D118736: [libc++] Guard std::ranges under _LIBCPP_HAS_NO_INCOMPLETE_RANGES.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/libcxx/ranges/has-no-incomplete-ranges.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
Quuxplusone wrote:
> ldionne wrote:
> > I don't think this test adds that much value -- instead, we should just add a CI configuration that tests without incomplete features, and also make sure not to needlessly mark tests as `UNSUPPORTED` (like we did for the span ones).
> We should definitely add a CI configuration that includes `libcpp-has-no-incomplete-ranges` and/or `libcpp-has-no-incomplete-format`. However, this test is the only test that actually tests the //point// of that config flag: that, when it is enabled, you //don't get// these features. Otherwise, we could just remove the macro from the entire codebase (or misspell it) and CI would never notice that the features had become enabled.
> (Like how right now `std::ranges::advance` is enabled: do we want it to be? Maybe we do, in which case this test is not harmless but actually testing something we //don't want//. But we should consciously decide that.)
Okay, I agree. Can we instead use a `.verify.cpp` test that does `namespace rng = std::ranges` and checks for an error? As it stands, the test is technically IFNDR.

Also, regarding testing a configuration without incomplete features, see https://reviews.llvm.org/D118740.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118736