[libcxx-commits] [libcxx] 4b3e0d2 - [libc++] Fix LWG3533 "Make `base() const&` consistent..."

Arthur O'Dwyer via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jan 26 16:43:43 PST 2022


Author: Arthur O'Dwyer
Date: 2022-01-26T19:38:39-05:00
New Revision: 4b3e0d2a7eb7cfc9bc6b8ec97fd42acbf6228614

URL: https://github.com/llvm/llvm-project/commit/4b3e0d2a7eb7cfc9bc6b8ec97fd42acbf6228614
DIFF: https://github.com/llvm/llvm-project/commit/4b3e0d2a7eb7cfc9bc6b8ec97fd42acbf6228614.diff

LOG: [libc++] Fix LWG3533 "Make `base() const&` consistent..."

Fixed in counted_iterator and transform_view::iterator.
The LWG issue also affected elements_view::iterator, but we haven't
implemented that one yet, and whoever does implement it will get
the fix for free if they just follow the working draft's wording.

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

Differential Revision: https://reviews.llvm.org/D117329

Added: 
    

Modified: 
    libcxx/docs/Status/Cxx2bIssues.csv
    libcxx/include/__iterator/counted_iterator.h
    libcxx/include/__ranges/transform_view.h
    libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp
    libcxx/test/std/ranges/range.adaptors/range.transform/types.h

Removed: 
    


################################################################################
diff  --git a/libcxx/docs/Status/Cxx2bIssues.csv b/libcxx/docs/Status/Cxx2bIssues.csv
index f949102e94b19..3dcb641a866bc 100644
--- a/libcxx/docs/Status/Cxx2bIssues.csv
+++ b/libcxx/docs/Status/Cxx2bIssues.csv
@@ -81,7 +81,7 @@
 `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|"

diff  --git a/libcxx/include/__iterator/counted_iterator.h b/libcxx/include/__iterator/counted_iterator.h
index 3ce88fadbe4d7..82d7adcfb02ea 100644
--- a/libcxx/include/__iterator/counted_iterator.h
+++ b/libcxx/include/__iterator/counted_iterator.h
@@ -96,7 +96,7 @@ class counted_iterator
   }
 
   _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_); }

diff  --git a/libcxx/include/__ranges/transform_view.h b/libcxx/include/__ranges/transform_view.h
index 0f53fbaa7e686..1506e8b2a7fed 100644
--- a/libcxx/include/__ranges/transform_view.h
+++ b/libcxx/include/__ranges/transform_view.h
@@ -195,9 +195,7 @@ class transform_view<_View, _Fn>::__iterator
     : __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_;
   }
 

diff  --git a/libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp b/libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp
index 57afdbfb1bd26..9d11485782c92 100644
--- a/libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp
+++ b/libcxx/test/std/iterators/predef.iterators/counted.iterator/base.pass.cpp
@@ -32,17 +32,18 @@ constexpr bool test() {
 
   {
     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*>);
@@ -50,8 +51,8 @@ constexpr bool test() {
 
   {
     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*>);
@@ -68,8 +69,8 @@ constexpr bool test() {
 
   {
     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*>&);
@@ -77,8 +78,8 @@ constexpr bool test() {
 
   {
     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*>&);
@@ -86,8 +87,8 @@ constexpr bool test() {
 
   {
     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*>&);

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp
index 80b8c92bf251a..23972b79ea498 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.transform/end.pass.cpp
@@ -32,50 +32,87 @@ 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;
 }

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp
index 26fc953023c81..6d54e9998e15c 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.transform/iterator/base.pass.cpp
@@ -17,31 +17,32 @@
 #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;
 }

diff  --git a/libcxx/test/std/ranges/range.adaptors/range.transform/types.h b/libcxx/test/std/ranges/range.adaptors/range.transform/types.h
index 80ffa6a0a67d0..64794c2262779 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.transform/types.h
+++ b/libcxx/test/std/ranges/range.adaptors/range.transform/types.h
@@ -46,8 +46,8 @@ struct ForwardView : std::ranges::view_base {
   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>);


        


More information about the libcxx-commits mailing list