[libc++] Fix signatures of counted_iterator::base(), transform_view::iterator::base().

[• Quuxplusone]

Summary

Drive-by stop calling .base() on test iterators in the test, and improve the transform_view::iterator/sentinel tests.

[ldionne]

LGTM, but I think we suffer from the same problem in other iterators, for example transform_view::iterator but probably others too. Any appetite for fixing all of them in this PR?

[• Quuxplusone]

Turns out that transform_view::iterator is the only other one, that we've already implemented, that doesn't require major other changes for C++20.

move_iterator and reverse_iterator are also affected but require major changes for C++20 that we haven't implemented yet.
filter_view::iterator is affected but we haven't implemented it yet.
lazy_split_view::iterator is affected but we haven't implemented it yet.
elements_view::iterator is affected but we haven't implemented it yet.

[• Quuxplusone]

Fix a test failure, and improve the transform_view::{iterator/sentinel} tests.

[• Quuxplusone]

Rebase to poke CI.

[• Quuxplusone]

Turns out this is LWG3533; so now we can mark that issue Complete.
CI is still green.

[Mordante]

In D117329#3263611, @Quuxplusone wrote:

Turns out this is LWG3533; so now we can mark that issue Complete.

Please add this to the commit message and mention [range.elements.iterator] has been omitted since we haven't implemented that yet.
(I expect the person implementing that to use the current wording in the Standard.)

LTGM!

libcxx/docs/Status/Cxx2bIssues.csv

 `3522 <https://wg21.link/LWG3522>`__,"Missing requirement on ``InputIterator`` template parameter for ``priority_queue`` constructors","June 2021","|Complete|","14.0","|ranges|"
 `3523 <https://wg21.link/LWG3523>`__,"``iota_view::sentinel`` is not always ``iota_view``'s sentinel","June 2021","","","|ranges|"
 `3526 <https://wg21.link/LWG3526>`__,"Return types of ``uses_allocator_construction_args`` unspecified","June 2021","",""
 `3527 <https://wg21.link/LWG3527>`__,"``uses_allocator_construction_args`` handles rvalue pairs of rvalue references incorrectly","June 2021","",""
 `3528 <https://wg21.link/LWG3528>`__,"``make_from_tuple`` can perform (the equivalent of) a C-style cast","June 2021","",""
 `3529 <https://wg21.link/LWG3529>`__,"``priority_queue(first, last)`` should construct ``c`` with ``(first, last)``","June 2021","",""
 `3530 <https://wg21.link/LWG3530>`__,"``BUILTIN-PTR-MEOW`` should not opt the type out of syntactic checks","June 2021","",""
 `3532 <https://wg21.link/LWG3532>`__,"``split_view<V, P>::inner-iterator<true>::operator++(int)`` should depend on ``Base``","June 2021","","","|ranges|"
-`3533 <https://wg21.link/LWG3533>`__,"Make ``base() const &`` consistent across iterator wrappers that supports ``input_iterators``","June 2021","","","|ranges|"
+`3533 <https://wg21.link/LWG3533>`__,"Make ``base() const &`` consistent across iterator wrappers that supports ``input_iterators``","June 2021","|Complete|","14.0","|ranges|"
 `3536 <https://wg21.link/LWG3536>`__,"Should ``chrono::from_stream()`` assign zero to duration for failure?","June 2021","","","|chrono|"
 `3539 <https://wg21.link/LWG3539>`__,"``format_to`` must not copy models of ``output_iterator<const charT&>``","June 2021","","","|format|"
 `3540 <https://wg21.link/LWG3540>`__,"§[format.arg] There should be no const in ``basic_format_arg(const T* p)``","June 2021","|Complete|","14.0","|format|"
 `3541 <https://wg21.link/LWG3541>`__,"``indirectly_readable_traits`` should be SFINAE-friendly for all types","June 2021","|Complete|","14.0","|ranges|"
 `3542 <https://wg21.link/LWG3542>`__,"``basic_format_arg`` mishandles ``basic_string_view`` with custom traits","June 2021","|Complete|","14.0","|format|"
 `3543 <https://wg21.link/LWG3543>`__,"Definition of when ``counted_iterators`` refer to the same sequence isn't quite right","June 2021","","","|ranges|"
 `3544 <https://wg21.link/LWG3544>`__,"``format-arg-store::args`` is unintentionally not exposition-only","June 2021","","","|format|"
 `3546 <https://wg21.link/LWG3546>`__,"``common_iterator``'s postfix-proxy is not quite right","June 2021","","","|ranges|"

libcxx/include/__iterator/counted_iterator.h

public:
   _LIBCPP_HIDE_FROM_ABI
   constexpr counted_iterator& operator=(const counted_iterator<_I2>& __other) {
     __current_ = __other.__current_;
     __count_ = __other.__count_;
     return *this;
   }
 
   _LIBCPP_HIDE_FROM_ABI
-  constexpr const _Iter& base() const& { return __current_; }
+  constexpr const _Iter& base() const& noexcept { return __current_; }
 
   _LIBCPP_HIDE_FROM_ABI
   constexpr _Iter base() && { return _VSTD::move(__current_); }
 
   _LIBCPP_HIDE_FROM_ABI
   constexpr iter_difference_t<_Iter> count() const noexcept { return __count_; }
 
   _LIBCPP_HIDE_FROM_ABI

libcxx/include/__ranges/transform_view.h

public:
   // `__iterator<false>` is ill-formed when `_Const` is false
   // (see http://wg21.link/class.copy.ctor#5).
   _LIBCPP_HIDE_FROM_ABI
   constexpr __iterator(__iterator<!_Const> __i)
     requires _Const && convertible_to<iterator_t<_View>, iterator_t<_Base>>
     : __parent_(__i.__parent_), __current_(_VSTD::move(__i.__current_)) {}
 
   _LIBCPP_HIDE_FROM_ABI
-  constexpr iterator_t<_Base> base() const&
-    requires copyable<iterator_t<_Base>>
-  {
+  constexpr const iterator_t<_Base>& base() const& noexcept {
     return __current_;
   }
 
   _LIBCPP_HIDE_FROM_ABI
   constexpr iterator_t<_Base> base() && {
     return _VSTD::move(__current_);
   }
 

libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp

struct InputOrOutputArchetype {
   InputOrOutputArchetype& operator++() { ++ptr; return *this; }
 };
 
 constexpr bool test() {
   int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
 
   {
     std::counted_iterator iter(cpp20_input_iterator<int*>{buffer}, 8);
-    assert(iter.base().base() == buffer);
-    assert(std::move(iter).base().base() == buffer);
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
+    ASSERT_NOEXCEPT(iter.base());
     ASSERT_SAME_TYPE(decltype(iter.base()), const cpp20_input_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), cpp20_input_iterator<int*>);
   }
 
   {
     std::counted_iterator iter(forward_iterator<int*>{buffer}, 8);
-    assert(iter.base() == forward_iterator<int*>{buffer});
-    assert(std::move(iter).base() == forward_iterator<int*>{buffer});
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const forward_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), forward_iterator<int*>);
   }
 
   {
     std::counted_iterator iter(contiguous_iterator<int*>{buffer}, 8);
-    assert(iter.base() == contiguous_iterator<int*>{buffer});
-    assert(std::move(iter).base() == contiguous_iterator<int*>{buffer});
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const contiguous_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), contiguous_iterator<int*>);
   }
 
   {
     std::counted_iterator iter(InputOrOutputArchetype{buffer}, 6);
     assert(iter.base().ptr == buffer);
     assert(std::move(iter).base().ptr == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const InputOrOutputArchetype&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), InputOrOutputArchetype);
   }
 
   {
     const std::counted_iterator iter(cpp20_input_iterator<int*>{buffer}, 8);
-    assert(iter.base().base() == buffer);
-    assert(std::move(iter).base().base() == buffer);
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const cpp20_input_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), const cpp20_input_iterator<int*>&);
   }
 
   {
     const std::counted_iterator iter(forward_iterator<int*>{buffer}, 7);
-    assert(iter.base() == forward_iterator<int*>{buffer});
-    assert(std::move(iter).base() == forward_iterator<int*>{buffer});
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const forward_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), const forward_iterator<int*>&);
   }
 
   {
     const std::counted_iterator iter(contiguous_iterator<int*>{buffer}, 6);
-    assert(iter.base() == contiguous_iterator<int*>{buffer});
-    assert(std::move(iter).base() == contiguous_iterator<int*>{buffer});
+    assert(base(iter.base()) == buffer);
+    assert(base(std::move(iter).base()) == buffer);
 
     ASSERT_SAME_TYPE(decltype(iter.base()), const contiguous_iterator<int*>&);
     ASSERT_SAME_TYPE(decltype(std::move(iter).base()), const contiguous_iterator<int*>&);
   }
 
   {
     const std::counted_iterator iter(InputOrOutputArchetype{buffer}, 6);
     assert(iter.base().ptr == buffer);

libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp

 template<class T>
 concept HasConstQualifiedEnd = requires(const T& t) { t.end(); };
 
 constexpr bool test() {
   {
     using TransformView = std::ranges::transform_view<ForwardView, PlusOneMutable>;
     static_assert(std::ranges::common_range<TransformView>);
     TransformView tv;
-    auto end = tv.end();
-    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<ForwardView>);
-    assert(base(end.base()) == globalBuff + 8);
+    auto it = tv.end();
+    using It = decltype(it);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&>(it).base()), const forward_iterator<int*>&);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&&>(it).base()), forward_iterator<int*>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&>(it).base()), const forward_iterator<int*>&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&&>(it).base()), const forward_iterator<int*>&);
+    assert(base(it.base()) == globalBuff + 8);
+    assert(base(std::move(it).base()) == globalBuff + 8);
     static_assert(!HasConstQualifiedEnd<TransformView>);
   }
   {
     using TransformView = std::ranges::transform_view<InputView, PlusOneMutable>;
     static_assert(!std::ranges::common_range<TransformView>);
     TransformView tv;
-    auto end = tv.end();
-    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<InputView>);
-    assert(base(base(end.base())) == globalBuff + 8);
+    auto sent = tv.end();
+    using Sent = decltype(sent);
+    ASSERT_SAME_TYPE(decltype(static_cast<Sent&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<Sent&&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const Sent&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const Sent&&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    assert(base(base(sent.base())) == globalBuff + 8);
+    assert(base(base(std::move(sent).base())) == globalBuff + 8);
     static_assert(!HasConstQualifiedEnd<TransformView>);
   }
   {
     using TransformView = std::ranges::transform_view<InputView, PlusOne>;
     static_assert(!std::ranges::common_range<TransformView>);
     TransformView tv;
-    auto end = tv.end();
-    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<InputView>);
-    assert(base(base(end.base())) == globalBuff + 8);
-    auto cend = std::as_const(tv).end();
-    ASSERT_SAME_TYPE(decltype(cend.base()), std::ranges::sentinel_t<const InputView>);
-    assert(base(base(cend.base())) == globalBuff + 8);
+    auto sent = tv.end();
+    using Sent = decltype(sent);
+    ASSERT_SAME_TYPE(decltype(static_cast<Sent&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<Sent&&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const Sent&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const Sent&&>(sent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    assert(base(base(sent.base())) == globalBuff + 8);
+    assert(base(base(std::move(sent).base())) == globalBuff + 8);
+
+    auto csent = std::as_const(tv).end();
+    using CSent = decltype(csent);
+    ASSERT_SAME_TYPE(decltype(static_cast<CSent&>(csent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<CSent&&>(csent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const CSent&>(csent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const CSent&&>(csent).base()), sentinel_wrapper<cpp20_input_iterator<int*>>);
+    assert(base(base(csent.base())) == globalBuff + 8);
+    assert(base(base(std::move(csent).base())) == globalBuff + 8);
   }
   {
     using TransformView = std::ranges::transform_view<MoveOnlyView, PlusOneMutable>;
     static_assert(std::ranges::common_range<TransformView>);
     TransformView tv;
-    auto end = tv.end();
-    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<MoveOnlyView>);
-    assert(end.base() == globalBuff + 8);
+    auto it = tv.end();
+    using It = decltype(it);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&&>(it).base()), int*);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&&>(it).base()), int* const&);
+    assert(base(it.base()) == globalBuff + 8);
+    assert(base(std::move(it).base()) == globalBuff + 8);
     static_assert(!HasConstQualifiedEnd<TransformView>);
   }
   {
     using TransformView = std::ranges::transform_view<MoveOnlyView, PlusOne>;
     static_assert(std::ranges::common_range<TransformView>);
     TransformView tv;
-    auto end = tv.end();
-    ASSERT_SAME_TYPE(decltype(end.base()), std::ranges::sentinel_t<MoveOnlyView>);
-    assert(end.base() == globalBuff + 8);
-    auto cend = std::as_const(tv).end();
-    ASSERT_SAME_TYPE(decltype(cend.base()), std::ranges::sentinel_t<const MoveOnlyView>);
-    assert(cend.base() == globalBuff + 8);
+    auto it = tv.end();
+    using It = decltype(it);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&&>(it).base()), int*);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&&>(it).base()), int* const&);
+    assert(base(it.base()) == globalBuff + 8);
+    assert(base(std::move(it).base()) == globalBuff + 8);
+
+    auto csent = std::as_const(tv).end();
+    using CSent = decltype(csent);
+    ASSERT_SAME_TYPE(decltype(static_cast<CSent&>(csent).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<CSent&&>(csent).base()), int*);
+    ASSERT_SAME_TYPE(decltype(static_cast<const CSent&>(csent).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const CSent&&>(csent).base()), int* const&);
+    assert(base(base(csent.base())) == globalBuff + 8);
+    assert(base(base(std::move(csent).base())) == globalBuff + 8);
   }
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp

 
 // transform_view::<iterator>::base
 
 #include <ranges>
 
 #include "test_macros.h"
 #include "../types.h"
 
-template<class It>
-concept HasBase = requires(It it) {
-  static_cast<It>(it).base();
-};
-
 constexpr bool test() {
   {
     using TransformView = std::ranges::transform_view<MoveOnlyView, PlusOneMutable>;
     TransformView tv;
-    auto begin = tv.begin();
-    ASSERT_SAME_TYPE(decltype(begin.base()), int*);
-    assert(begin.base() == globalBuff);
-    ASSERT_SAME_TYPE(decltype(std::move(begin).base()), int*);
-    assert(std::move(begin).base() == globalBuff);
+    auto it = tv.begin();
+    using It = decltype(it);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&&>(it).base()), int*);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&>(it).base()), int* const&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&&>(it).base()), int* const&);
+    ASSERT_NOEXCEPT(it.base());
+    assert(base(it.base()) == globalBuff);
+    assert(base(std::move(it).base()) == globalBuff);
   }
   {
     using TransformView = std::ranges::transform_view<InputView, PlusOneMutable>;
     TransformView tv;
-    auto begin = tv.begin();
-    static_assert(!HasBase<decltype(begin)&>);
-    static_assert(HasBase<decltype(begin)&&>);
-    static_assert(!HasBase<const decltype(begin)&>);
-    static_assert(!HasBase<const decltype(begin)&&>);
-    std::same_as<cpp20_input_iterator<int *>> auto it = std::move(begin).base();
-    assert(base(it) == globalBuff);
+    auto it = tv.begin();
+    using It = decltype(it);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&>(it).base()), const cpp20_input_iterator<int*>&);
+    ASSERT_SAME_TYPE(decltype(static_cast<It&&>(it).base()), cpp20_input_iterator<int*>);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&>(it).base()), const cpp20_input_iterator<int*>&);
+    ASSERT_SAME_TYPE(decltype(static_cast<const It&&>(it).base()), const cpp20_input_iterator<int*>&);
+    ASSERT_NOEXCEPT(it.base());
+    assert(base(it.base()) == globalBuff);
+    assert(base(std::move(it).base()) == globalBuff);
   }
   return true;
 }
 
 int main(int, char**) {
   test();
   static_assert(test());
 
   return 0;
 }

libcxx/test/std/ranges/range.adaptors/range.transform/types.h

 static_assert(std::copyable<CopyableView>);
 
 using ForwardIter = forward_iterator<int*>;
 struct ForwardView : std::ranges::view_base {
   int *ptr_;
   constexpr explicit ForwardView(int* ptr = globalBuff) : ptr_(ptr) {}
   constexpr ForwardView(ForwardView&&) = default;
   constexpr ForwardView& operator=(ForwardView&&) = default;
-  constexpr auto begin() const { return ForwardIter(ptr_); }
-  constexpr auto end() const { return ForwardIter(ptr_ + 8); }
+  constexpr auto begin() const { return forward_iterator<int*>(ptr_); }
+  constexpr auto end() const { return forward_iterator<int*>(ptr_ + 8); }
 };
 static_assert(std::ranges::view<ForwardView>);
 static_assert(std::ranges::forward_range<ForwardView>);
 
 using ForwardRange = test_common_range<forward_iterator>;
 static_assert(!std::ranges::view<ForwardRange>);
 static_assert( std::ranges::forward_range<ForwardRange>);