[libcxx-commits] [PATCH] D108575: [libcxx] Define insert_iterator::iter with ranges::iterator_t

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

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/iterators/predef.iterators/insert.iterators/insert.iterator/cxx20_iter_member.pass.cpp:39
+    auto it = std::insert_iterator<NoIteratorAlias>(c, c.data_);
+    ASSERT_SAME_TYPE(decltype(std::inserter(c, c.data_)), std::insert_iterator<NoIteratorAlias>);
+    *it++ = 1 + half;  // test that RHS is still implicitly converted to _Container::value_type
----------------
jloser wrote:
> Should we just use `static_assert` with `std::is_same_v` since this test file only supports C++20 anyway (meaning there isn't a need to route through `ASSERT_SAME_TYPE`)?
I think the question is not quite settled globally, but personally, I prefer `ASSERT_SAME_TYPE` because
- it is shorter, and
- it "always works" — doesn't require making any judgment calls about which C++ versions the test supports — and
- it won't require refactoring five years down the road when the test gets extended to other C++ versions (this happens more often than you might think).

Hypothetically, if we //were// to go the "always use the latest and greatest possible, no software-engineering considerations" route, then we'd also have to consider `static_assert` with `std::same_as` ("since this test file only supports C++20 with concepts+ranges anyway").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108575