[libcxx-commits] [PATCH] D117369: [libcxx][test] `unordered_meow` iterators are not portably non-bidi

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added a comment.
This revision now requires changes to proceed.

We should pick //one// mechanism (`#ifdef _MSVC_STL_VERSION` or `LIBCPP_STATIC_ASSERT`) and stick to it. But LGTM besides that.



================
Comment at: libcxx/test/std/containers/unord/unord.map/iterator_concept_conformance.compile.pass.cpp:31-32
 static_assert(std::sentinel_for<iterator, const_iterator>);
-static_assert(!std::sentinel_for<iterator, local_iterator>);
-static_assert(!std::sentinel_for<iterator, const_local_iterator>);
+LIBCPP_STATIC_ASSERT(!std::sentinel_for<iterator, local_iterator>);
+LIBCPP_STATIC_ASSERT(!std::sentinel_for<iterator, const_local_iterator>);
 static_assert(!std::sized_sentinel_for<iterator, iterator>);
----------------
Throughout: I think we should eliminate all tests for `!{sized_,}sentinel_for<X, Y>` where X and Y are unrelated types. Basically this file should become
```
static_assert(std::forward_iterator<iterator>);
LIBCPP_STATIC_ASSERT(!std::bidirectional_iterator<iterator>);
static_assert(!std::indirectly_writable<iterator, value_type>);
static_assert(std::sentinel_for<iterator, iterator>);
static_assert(std::sentinel_for<iterator, const_iterator>);
static_assert(!std::sized_sentinel_for<iterator, iterator>);
static_assert(!std::sized_sentinel_for<iterator, const_iterator>);
static_assert(std::indirectly_movable<iterator, std::pair<int, int>*>);
static_assert(!std::indirectly_movable_storable<iterator, std::pair<int, int>*>);
static_assert(!std::indirectly_swappable<iterator, iterator>);

static_assert(std::forward_iterator<const_iterator>);
LIBCPP_STATIC_ASSERT(!std::bidirectional_iterator<const_iterator>);
static_assert(!std::indirectly_writable<const_iterator, value_type>);
static_assert(std::sentinel_for<const_iterator, iterator>);
static_assert(std::sentinel_for<const_iterator, const_iterator>);
static_assert(!std::sized_sentinel_for<const_iterator, iterator>);
static_assert(!std::sized_sentinel_for<const_iterator, const_iterator>);
static_assert(std::indirectly_movable<const_iterator, std::pair<int, int>*>);
static_assert(!std::indirectly_movable_storable<const_iterator, std::pair<int, int>*>);
static_assert(!std::indirectly_swappable<const_iterator, const_iterator>);
```
and then the same set of assertions but with `s/iterator/local_iterator/g`. Ditto in the other test files.


================
Comment at: libcxx/test/std/containers/unord/unord.set/iterator_concept_conformance.compile.pass.cpp:25-30
+#ifdef _MSVC_STL_VERSION
+static_assert(std::bidirectional_iterator<iterator>);
+static_assert(!std::random_access_iterator<iterator>);
+#else // ^^^ MSVC STL // other vvv
 static_assert(!std::bidirectional_iterator<iterator>);
+#endif // _MSVC_STL_VERSION
----------------
We should take the same approach here as in the other test files you're touching.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117369