Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] P2770R0: "Stashing stashing iterators for proper flattening" #66033

Merged
merged 20 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
600a282
[libc++] P2770R0: "Stashing stashing iterators for proper flattening"
JMazurkiewicz Sep 8, 2023
743b01d
Merge branch 'main' into libcxx/ranges/stashing2
JMazurkiewicz Nov 27, 2023
f601c07
Enable `__as_lvalue` in >C++03 modes
JMazurkiewicz Nov 27, 2023
be16b57
Split `as-lvalue.verify.cpp`
JMazurkiewicz Nov 27, 2023
181cd85
Add missing `explicit` specifier in `__iterator(_Parent& __parent)`
JMazurkiewicz Nov 27, 2023
d11d81c
Restore old `__sentinel<C>` friendship in `join_view::_sentinel`
JMazurkiewicz Nov 27, 2023
03454c2
Un-experimental more `join_view` related tests
JMazurkiewicz Nov 27, 2023
7de9a77
Make `join_view::__iterator::__outer_` private
JMazurkiewicz Nov 27, 2023
f256e1f
Add test with code from LWG-3698
JMazurkiewicz Nov 27, 2023
1f00358
Whitespace amendments
JMazurkiewicz Nov 27, 2023
d5d0610
Add extra static assertion for `[range.join.iterator] Note 1`
JMazurkiewicz Nov 27, 2023
3861ffa
Fix comment: `!forward_range<iterator_t<Base>>` -> `!forward_range<Ba…
JMazurkiewicz Nov 27, 2023
0e92781
Make `as-lvalue.pass.cpp` C++11 friendly
JMazurkiewicz Nov 27, 2023
54c31fc
Add `return 0;` in LWG-3698 test
JMazurkiewicz Nov 29, 2023
62fa3a7
Revert "Make `as-lvalue.pass.cpp` C++11 friendly"
JMazurkiewicz Nov 29, 2023
aebbbac
Make `as-lvalue.pass.cpp` C++14 friendly
JMazurkiewicz Nov 29, 2023
ca61a70
Test `iterator(Parent& parent, OuterIter outer)` constructor
JMazurkiewicz Dec 5, 2023
024b9e2
Test `iterator(Parent& parent)` constructor
JMazurkiewicz Dec 5, 2023
857aae1
Merge branch 'main' into libcxx/ranges/stashing
JMazurkiewicz Dec 5, 2023
710398a
Address GCC's complaints
JMazurkiewicz Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions libcxx/docs/Status/Cxx23.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Paper Status
clang doesn't issue a diagnostic for deprecated using template declarations.
.. [#note-P2520R0] P2520R0: Libc++ implemented this paper as a DR in C++20 as well.
.. [#note-P2711R1] P2711R1: ``join_with_view`` hasn't been done yet since this type isn't implemented yet.
.. [#note-P2770R0] P2770R0: ``join_with_view`` hasn't been done yet since this type isn't implemented yet.
.. [#note-P2693R1] P2693R1: The formatter for ``std::thread::id`` is implemented.
The formatter for ``stacktrace`` is not implemented, since ``stacktrace`` is
not implemented yet.
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Papers.csv
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
"`P2708R1 <https://wg21.link/P2708R1>`__","LWG", "No Further Fundamentals TSes", "November 2022","|Nothing to do|","",""
"","","","","","",""
"`P0290R4 <https://wg21.link/P0290R4>`__","LWG", "``apply()`` for ``synchronized_value<T>``","February 2023","","","|concurrency TS|"
"`P2770R0 <https://wg21.link/P2770R0>`__","LWG", "Stashing stashing ``iterators`` for proper flattening","February 2023","|In Progress|","","|ranges|"
"`P2770R0 <https://wg21.link/P2770R0>`__","LWG", "Stashing stashing ``iterators`` for proper flattening","February 2023","|Partial| [#note-P2770R0]_","","|ranges|"
ldionne marked this conversation as resolved.
Show resolved Hide resolved
"`P2164R9 <https://wg21.link/P2164R9>`__","LWG", "``views::enumerate``","February 2023","","","|ranges|"
"`P2711R1 <https://wg21.link/P2711R1>`__","LWG", "Making multi-param constructors of ``views`` ``explicit``","February 2023","|In Progress| [#note-P2711R1]_","","|ranges|"
"`P2609R3 <https://wg21.link/P2609R3>`__","LWG", "Relaxing Ranges Just A Smidge","February 2023","","","|ranges|"
Expand Down
1 change: 0 additions & 1 deletion libcxx/docs/UsingLibcxx.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ when ``-fexperimental-library`` is passed:
* ``std::stop_token``, ``std::stop_source`` and ``std::stop_callback``
* ``std::jthread``
* ``std::chrono::tzdb`` and related time zone functionality
* ``std::ranges::join_view``

.. warning::
Experimental libraries are experimental.
Expand Down
1 change: 1 addition & 0 deletions libcxx/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,7 @@ set(files
__type_traits/void_t.h
__undef_macros
__utility/as_const.h
__utility/as_lvalue.h
__utility/auto_cast.h
__utility/cmp.h
__utility/convert_to_integral.h
Expand Down
132 changes: 81 additions & 51 deletions libcxx/include/__ranges/join_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
#include <__ranges/view_interface.h>
#include <__type_traits/common_type.h>
#include <__type_traits/maybe_const.h>
#include <__utility/as_lvalue.h>
#include <__utility/empty.h>
#include <__utility/forward.h>
#include <optional>

Expand All @@ -41,10 +43,7 @@

_LIBCPP_BEGIN_NAMESPACE_STD

// Note: `join_view` is still marked experimental because there is an ABI-breaking change that affects `join_view` in
// the pipeline (https://isocpp.org/files/papers/D2770R0.html).
// TODO: make `join_view` non-experimental once D2770 is implemented.
#if _LIBCPP_STD_VER >= 20 && defined(_LIBCPP_ENABLE_EXPERIMENTAL)
#if _LIBCPP_STD_VER >= 20

namespace ranges {
template<class>
Expand Down Expand Up @@ -84,11 +83,16 @@ namespace ranges {
template <class>
friend struct std::__segmented_iterator_traits;

static constexpr bool _UseCache = !is_reference_v<_InnerRange>;
using _Cache = _If<_UseCache, __non_propagating_cache<remove_cvref_t<_InnerRange>>, __empty_cache>;
_LIBCPP_NO_UNIQUE_ADDRESS _Cache __cache_;
_LIBCPP_NO_UNIQUE_ADDRESS _View __base_ = _View();

static constexpr bool _UseOuterCache = !forward_range<_View>;
using _OuterCache = _If<_UseOuterCache, __non_propagating_cache<iterator_t<_View>>, __empty_cache>;
_LIBCPP_NO_UNIQUE_ADDRESS _OuterCache __outer_;

static constexpr bool _UseInnerCache = !is_reference_v<_InnerRange>;
using _InnerCache = _If<_UseInnerCache, __non_propagating_cache<remove_cvref_t<_InnerRange>>, __empty_cache>;
_LIBCPP_NO_UNIQUE_ADDRESS _InnerCache __inner_;
ldionne marked this conversation as resolved.
Show resolved Hide resolved

public:
_LIBCPP_HIDE_FROM_ABI
join_view() requires default_initializable<_View> = default;
Expand All @@ -105,16 +109,22 @@ namespace ranges {

_LIBCPP_HIDE_FROM_ABI
constexpr auto begin() {
constexpr bool __use_const = __simple_view<_View> &&
is_reference_v<range_reference_t<_View>>;
return __iterator<__use_const>{*this, ranges::begin(__base_)};
if constexpr (forward_range<_View>) {
constexpr bool __use_const = __simple_view<_View> &&
is_reference_v<range_reference_t<_View>>;
return __iterator<__use_const>{*this, ranges::begin(__base_)};
} else {
__outer_.__emplace(ranges::begin(__base_));
return __iterator<false>{*this};
}
}

template<class _V2 = _View>
_LIBCPP_HIDE_FROM_ABI
constexpr auto begin() const
requires input_range<const _V2> &&
is_reference_v<range_reference_t<const _V2>>
requires forward_range<const _V2> &&
is_reference_v<range_reference_t<const _V2>> &&
input_range<range_reference_t<const _V2>>
{
return __iterator<true>{*this, ranges::begin(__base_)};
}
Expand All @@ -134,13 +144,12 @@ namespace ranges {
template<class _V2 = _View>
_LIBCPP_HIDE_FROM_ABI
constexpr auto end() const
requires input_range<const _V2> &&
is_reference_v<range_reference_t<const _V2>>
requires forward_range<const _V2> &&
is_reference_v<range_reference_t<const _V2>> &&
input_range<range_reference_t<const _V2>>
{
using _ConstInnerRange = range_reference_t<const _View>;
if constexpr (forward_range<const _View> &&
is_reference_v<_ConstInnerRange> &&
forward_range<_ConstInnerRange> &&
if constexpr (forward_range<_ConstInnerRange> &&
common_range<const _View> &&
common_range<_ConstInnerRange>) {
return __iterator<true>{*this, ranges::end(__base_)};
Expand All @@ -154,11 +163,10 @@ namespace ranges {
requires view<_View> && input_range<range_reference_t<_View>>
template<bool _Const>
struct join_view<_View>::__sentinel {
template<bool>
friend struct __sentinel;

private:
using _Parent = __maybe_const<_Const, join_view<_View>>;
friend join_view;
ldionne marked this conversation as resolved.
Show resolved Hide resolved

using _Parent = __maybe_const<_Const, join_view>;
using _Base = __maybe_const<_Const, _View>;
sentinel_t<_Base> __end_ = sentinel_t<_Base>();

Expand All @@ -179,7 +187,7 @@ namespace ranges {
requires sentinel_for<sentinel_t<_Base>, iterator_t<__maybe_const<_OtherConst, _View>>>
_LIBCPP_HIDE_FROM_ABI
friend constexpr bool operator==(const __iterator<_OtherConst>& __x, const __sentinel& __y) {
return __x.__outer_ == __y.__end_;
return __x.__get_outer() == __y.__end_;
}
};

Expand All @@ -191,9 +199,7 @@ namespace ranges {
template<bool _Const>
struct join_view<_View>::__iterator final
: public __join_view_iterator_category<__maybe_const<_Const, _View>> {

template<bool>
friend struct __iterator;
friend join_view;
ldionne marked this conversation as resolved.
Show resolved Hide resolved

template <class>
friend struct std::__segmented_iterator_traits;
Expand All @@ -209,21 +215,24 @@ namespace ranges {

static constexpr bool __ref_is_glvalue = is_reference_v<range_reference_t<_Base>>;

static constexpr bool _OuterPresent = forward_range<_Base>;
using _OuterType = _If<_OuterPresent, _Outer, std::__empty>;
ldionne marked this conversation as resolved.
Show resolved Hide resolved

public:
_Outer __outer_ = _Outer();
_LIBCPP_NO_UNIQUE_ADDRESS _OuterType __outer_ = _OuterType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this private. I suspect you will need __get_outer() to be public (or to do some befriending of a few classes, which would be preferable to making it public).


private:
optional<_Inner> __inner_;
_Parent *__parent_ = nullptr;

_LIBCPP_HIDE_FROM_ABI
constexpr void __satisfy() {
for (; __outer_ != ranges::end(__parent_->__base_); ++__outer_) {
auto&& __inner = [&]() -> auto&& {
for (; __get_outer() != ranges::end(__parent_->__base_); ++__get_outer()) {
auto&& __inner = [this]() -> auto&& {
if constexpr (__ref_is_glvalue)
return *__outer_;
return *__get_outer();
else
return __parent_->__cache_.__emplace_from([&]() -> decltype(auto) { return *__outer_; });
return __parent_->__inner_.__emplace_from([&]() -> decltype(auto) { return *__get_outer(); });
}();
__inner_ = ranges::begin(__inner);
if (*__inner_ != ranges::end(__inner))
Expand All @@ -234,8 +243,37 @@ namespace ranges {
__inner_.reset();
}

_LIBCPP_HIDE_FROM_ABI constexpr _Outer& __get_outer() {
if constexpr (forward_range<_Base>) {
return __outer_;
} else {
return *__parent_->__outer_;
}
}

_LIBCPP_HIDE_FROM_ABI constexpr const _Outer& __get_outer() const {
if constexpr (forward_range<_Base>) {
return __outer_;
} else {
return *__parent_->__outer_;
}
}

_LIBCPP_HIDE_FROM_ABI constexpr __iterator(_Parent& __parent, _Outer __outer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add tests for these constructors? You removed the test in libcxx/test/std/ranges/range.adaptors/range.join.view/iterator/ctor.parent.outer.pass.cpp but I don't think you added any for the new ctors. Note that those are exposition only ctors so the tests could go in libcxx/test/libcxx/<...> instead of libcxx/test/std/<...> -- or if you prefer you can put them in libcxx/test/std but mark them with REQUIRES: stdlib=libc++.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this has been addressed (unless I missed it)?

requires forward_range<_Base>
: __outer_(std::move(__outer)), __parent_(std::addressof(__parent)) {
__satisfy();
}

_LIBCPP_HIDE_FROM_ABI constexpr __iterator(_Parent& __parent)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_LIBCPP_HIDE_FROM_ABI constexpr __iterator(_Parent& __parent)
_LIBCPP_HIDE_FROM_ABI constexpr explicit __iterator(_Parent& __parent)

requires(!forward_range<_Base>)
: __parent_(std::addressof(__parent)) {
__satisfy();
}

_LIBCPP_HIDE_FROM_ABI constexpr __iterator(_Parent* __parent, _Outer __outer, _Inner __inner)
: __outer_(std::move(__outer)), __inner_(std::move(__inner)), __parent_(__parent) {}
requires forward_range<_Base>
: __outer_(std::move(__outer)), __inner_(std::move(__inner)), __parent_(__parent) {}

public:
using iterator_concept = _If<
Expand All @@ -254,15 +292,7 @@ namespace ranges {
using difference_type = common_type_t<
range_difference_t<_Base>, range_difference_t<range_reference_t<_Base>>>;

_LIBCPP_HIDE_FROM_ABI
__iterator() requires default_initializable<_Outer> = default;

_LIBCPP_HIDE_FROM_ABI
constexpr __iterator(_Parent& __parent, _Outer __outer)
: __outer_(std::move(__outer))
, __parent_(std::addressof(__parent)) {
__satisfy();
}
_LIBCPP_HIDE_FROM_ABI __iterator() = default;

_LIBCPP_HIDE_FROM_ABI
constexpr __iterator(__iterator<!_Const> __i)
Expand All @@ -287,14 +317,14 @@ namespace ranges {

_LIBCPP_HIDE_FROM_ABI
constexpr __iterator& operator++() {
auto&& __inner = [&]() -> auto&& {
auto __get_inner_range = [&]() -> decltype(auto) {
if constexpr (__ref_is_glvalue)
return *__outer_;
return *__get_outer();
else
return *__parent_->__cache_;
}();
if (++*__inner_ == ranges::end(__inner)) {
++__outer_;
return *__parent_->__inner_;
};
if (++*__inner_ == ranges::end(ranges::__as_lvalue(__get_inner_range()))) {
JMazurkiewicz marked this conversation as resolved.
Show resolved Hide resolved
++__get_outer();
__satisfy();
}
return *this;
Expand Down Expand Up @@ -324,11 +354,11 @@ namespace ranges {
common_range<range_reference_t<_Base>>
{
if (__outer_ == ranges::end(__parent_->__base_))
__inner_ = ranges::end(*--__outer_);
__inner_ = ranges::end(ranges::__as_lvalue(*--__outer_));

// Skip empty inner ranges when going backwards.
while (*__inner_ == ranges::begin(*__outer_)) {
__inner_ = ranges::end(*--__outer_);
while (*__inner_ == ranges::begin(ranges::__as_lvalue(*__outer_))) {
__inner_ = ranges::end(ranges::__as_lvalue(*--__outer_));
}

--*__inner_;
Expand All @@ -350,7 +380,7 @@ namespace ranges {
_LIBCPP_HIDE_FROM_ABI
friend constexpr bool operator==(const __iterator& __x, const __iterator& __y)
requires __ref_is_glvalue &&
equality_comparable<iterator_t<_Base>> &&
forward_range<_Base> &&
equality_comparable<iterator_t<range_reference_t<_Base>>>
{
return __x.__outer_ == __y.__outer_ && __x.__inner_ == __y.__inner_;
Expand Down Expand Up @@ -436,7 +466,7 @@ struct __segmented_iterator_traits<_JoinViewIterator> {
}
};

#endif // #if _LIBCPP_STD_VER >= 20 && defined(_LIBCPP_ENABLE_EXPERIMENTAL)
#endif // #if _LIBCPP_STD_VER >= 20

_LIBCPP_END_NAMESPACE_STD

Expand Down
39 changes: 39 additions & 0 deletions libcxx/include/__utility/as_lvalue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// -*- C++ -*-
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___UTILITY_AS_LVALUE_H
#define _LIBCPP___UTILITY_AS_LVALUE_H

#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

_LIBCPP_PUSH_MACROS
#include <__undef_macros>

_LIBCPP_BEGIN_NAMESPACE_STD

#if _LIBCPP_STD_VER >= 20

namespace ranges {
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI constexpr _Tp& __as_lvalue(_LIBCPP_LIFETIMEBOUND _Tp&& __t) {
return static_cast<_Tp&>(__t);
}
} // namespace ranges

#endif // _LIBCPP_STD_VER >= 20
JMazurkiewicz marked this conversation as resolved.
Show resolved Hide resolved

_LIBCPP_END_NAMESPACE_STD

_LIBCPP_POP_MACROS

#endif // _LIBCPP___UTILITY_AS_LVALUE_H
1 change: 1 addition & 0 deletions libcxx/include/module.modulemap.in
Original file line number Diff line number Diff line change
Expand Up @@ -2041,6 +2041,7 @@ module std_private_type_traits_unwrap_ref [system
module std_private_type_traits_void_t [system] { header "__type_traits/void_t.h" }

module std_private_utility_as_const [system] { header "__utility/as_const.h" }
module std_private_utility_as_lvalue [system] { header "__utility/as_lvalue.h" }
module std_private_utility_auto_cast [system] {
header "__utility/auto_cast.h"
export std_private_type_traits_decay
Expand Down
8 changes: 8 additions & 0 deletions libcxx/include/regex
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ public:
typedef const value_type* pointer;
typedef const value_type& reference;
typedef forward_iterator_tag iterator_category;
typedef input_iterator_tag iterator_concept; // since C++20

regex_iterator();
regex_iterator(BidirectionalIterator a, BidirectionalIterator b,
Expand Down Expand Up @@ -737,6 +738,7 @@ public:
typedef const value_type* pointer;
typedef const value_type& reference;
typedef forward_iterator_tag iterator_category;
typedef input_iterator_tag iterator_concept; // since C++20

regex_token_iterator();
regex_token_iterator(BidirectionalIterator a, BidirectionalIterator b,
Expand Down Expand Up @@ -6407,6 +6409,9 @@ public:
typedef const value_type* pointer;
typedef const value_type& reference;
typedef forward_iterator_tag iterator_category;
#if _LIBCPP_STD_VER >= 20
JMazurkiewicz marked this conversation as resolved.
Show resolved Hide resolved
typedef input_iterator_tag iterator_concept;
#endif

private:
_BidirectionalIterator __begin_;
Expand Down Expand Up @@ -6542,6 +6547,9 @@ public:
typedef const value_type* pointer;
typedef const value_type& reference;
typedef forward_iterator_tag iterator_category;
#if _LIBCPP_STD_VER >= 20
typedef input_iterator_tag iterator_concept;
#endif

private:
typedef regex_iterator<_BidirectionalIterator, _CharT, _Traits> _Position;
Expand Down
1 change: 1 addition & 0 deletions libcxx/include/utility
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ template <class T>
#include <__assert> // all public C++ headers provide the assertion handler
#include <__config>
#include <__utility/as_const.h>
#include <__utility/as_lvalue.h>
#include <__utility/auto_cast.h>
#include <__utility/cmp.h>
#include <__utility/declval.h>
Expand Down
Loading
Loading