[libcxx-commits] [PATCH] D97162: [libcxx] adds std::ranges::swap, std::swappable, and std::swappable_with

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb added inline comments.


================
Comment at: libcxx/test/std/concepts/lang/swappable_with.compile.pass.cpp:593
+
+#ifndef _LIBCPP_HAS_NO_THREADS
+static_assert(!check_swappable_with<std::lock_guard<std::mutex>,
----------------
ldionne wrote:
> cjdb wrote:
> > Quuxplusone wrote:
> > > cjdb wrote:
> > > > ldionne wrote:
> > > > > EricWF wrote:
> > > > > > Can we write these tests in a way that makes them non-threading dependent.
> > > > > > 
> > > > > I don't think so since he's explicitly trying to exercise these types, which aren't provided when `_LIBCPP_HAS_NO_THREADS`. Better would be to have a test-suite version of the macro (to express the fact that the test suite supports being used when threads are not available), but that's for a different patch.
> > > > How about I do that patch just before announcing P0898 is implemented?
> > > Peanut gallery says: Other alternatives include
> > > 
> > > - test a made-up `struct Immobile` instead of specifically `std::mutex`; we can assume that `std::mutex` is equally immobile
> > > 
> > > - move tests of `std::mutex`'s own properties into test/std/thread/thread.mutex/ (because if someone were somehow to break the immobility of `std::mutex`, I'd hope that it's the //mutex// tests that fail, not some random concept's tests)
> > > 
> > > But I have no particular stake in the outcome.
> > Reason for having `std::mutex` over `immobile` is because @ldionne asked if I could test some standard types too, and I couldn't think of any non-thread based nonmovable types at the time. @EricWF and I agreed to change the `mutex` ones to some streams.
> I think you'll have the same problem with `_LIBCPP_HAS_NO_LOCALIZATION` now, unless I'm mistaken. You could move the test to the mutex tests instead if you wanted. Or remove those - they do provide nice testing coverage but they're not worth creating a ruckus over IMO!
Okay, so that's why CI failed this time, thanks for the clarity there :-)

In that case, I'll take Arthur's suggestion and make an immobile type, and in a later patch, make a test for thread and stream types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97162