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

Implement P2328R1 join_view Should Join All Views Of Ranges #2038

Merged
merged 22 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
92ce96e
Implement P2328R1 join_view should join all ranges
miscco Jul 2, 2021
49bf9a3
Specialize `_Non_propagating_cache` and fix test
miscco Jul 3, 2021
5e74f42
More tests
miscco Jul 3, 2021
8041fc4
We should rela test requirements
miscco Jul 3, 2021
351f538
More test coverage and internal testing
miscco Jul 4, 2021
72f392c
Address more review comments
miscco Jul 6, 2021
e283371
Properly destroy existing element in _Emplace_deref
miscco Jul 15, 2021
f546493
Further simplify _Non_propagating_cache for trivially_destructible types
miscco Jul 15, 2021
c2bc106
More review comments
miscco Jul 18, 2021
7b426da
Require `is_default_constructible_v` for the specialization of `_Non_…
miscco Jul 18, 2021
36cd233
Merge branch 'main' into P2328-join-view
miscco Jul 20, 2021
11f44fa
Use _CONSTEXPR20
miscco Jul 20, 2021
03c80bc
Update tests
miscco Jul 21, 2021
48ee23a
Merge branch 'main' into P2328-join-view
miscco Aug 4, 2021
317fe66
Address review comments
miscco Aug 4, 2021
2075c52
Update tests/std/tests/P0896R4_views_join/test.cpp
CaseyCarter Aug 4, 2021
3787596
Remove "dummy" union members not needed for C++20
CaseyCarter Aug 5, 2021
f9a7a6a
Enable join_view to handle immovable types
miscco Aug 5, 2021
52d36e1
Merge branch 'main' into P2328-join-view
miscco Aug 6, 2021
0c54f03
Hijack the `_Not_quite_object::_Construct_tag` to avoid conversions
miscco Aug 6, 2021
b3925ac
Remove unnecessary `const` on a nameless value parameter.
StephanTLavavej Aug 6, 2021
4a7ba61
Remove `|| defined(MSVC_INTERNAL_TESTING)`.
StephanTLavavej Aug 9, 2021
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
118 changes: 101 additions & 17 deletions stl/inc/ranges
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ namespace ranges {
&& same_as<sentinel_t<_Rng>, sentinel_t<const _Rng>>;

template <class _Ty>
concept _Copy_constructible_object = copy_constructible<_Ty> && is_object_v<_Ty>;
concept _Copy_constructible_object = copy_constructible<_Ty> && _Destructible_object<_Ty>;

template <class _It>
concept _Has_arrow = input_iterator<_It>
Expand Down Expand Up @@ -408,7 +408,6 @@ namespace ranges {

private:
union {
_Nontrivial_dummy_type _Dummy;
_Ty _Val;
};
bool _Engaged;
Expand Down Expand Up @@ -475,7 +474,7 @@ namespace ranges {
template <movable _Ty>
class _Defaultabox { // a simplified optional that augments movable types with default-constructibility
public:
constexpr _Defaultabox() noexcept : _Dummy{} {}
constexpr _Defaultabox() noexcept {}

// clang-format off
~_Defaultabox() requires is_trivially_destructible_v<_Ty> = default;
Expand Down Expand Up @@ -611,7 +610,6 @@ namespace ranges {

private:
union {
_Nontrivial_dummy_type _Dummy;
_Ty _Val;
};
bool _Engaged = false;
Expand Down Expand Up @@ -654,6 +652,86 @@ namespace ranges {
/* [[no_unique_address]] */ _Ty _Value{};
};

template <_Destructible_object _Ty>
class _Non_propagating_cache { // a simplified optional that resets on copy / move
public:
constexpr _Non_propagating_cache() noexcept {}

constexpr ~_Non_propagating_cache() {
if (_Engaged) {
_Val.~_Ty();
}
}

// clang-format off
~_Non_propagating_cache() requires is_trivially_destructible_v<_Ty> = default;
// clang-format on

constexpr _Non_propagating_cache(const _Non_propagating_cache&) noexcept {}
miscco marked this conversation as resolved.
Show resolved Hide resolved

constexpr _Non_propagating_cache(_Non_propagating_cache&& _Other) noexcept {
if (_Other._Engaged) {
miscco marked this conversation as resolved.
Show resolved Hide resolved
_Other._Val.~_Ty();
_Other._Engaged = false;
}
}

constexpr _Non_propagating_cache& operator=(const _Non_propagating_cache& _Other) noexcept {
if (_STD addressof(_Other) == this) {
return *this;
}

if (_Engaged) {
_Val.~_Ty();
_Engaged = false;
}

return *this;
}

constexpr _Non_propagating_cache& operator=(_Non_propagating_cache&& _Other) noexcept {
if (_Engaged) {
_Val.~_Ty();
_Engaged = false;
}

if (_Other._Engaged) {
_Other._Val.~_Ty();
_Other._Engaged = false;
}

return *this;
}

_NODISCARD constexpr _Ty& operator*() noexcept {
_STL_INTERNAL_CHECK(_Engaged);
return _Val;
}
_NODISCARD constexpr const _Ty& operator*() const noexcept {
_STL_INTERNAL_CHECK(_Engaged);
return _Val;
}

template <class... _Types>
constexpr _Ty& _Emplace(_Types&&... _Args) noexcept(is_nothrow_constructible_v<_Ty, _Types...>) {
if (_Engaged) {
_Val.~_Ty();
_Engaged = false;
}

_Construct_in_place(_Val, _STD forward<_Types>(_Args)...);
_Engaged = true;

return _Val;
}

private:
union {
_Ty _Val;
};
bool _Engaged = false;
miscco marked this conversation as resolved.
Show resolved Hide resolved
};

// clang-format off
template <class _Ty>
requires is_object_v<_Ty>
Expand Down Expand Up @@ -2873,15 +2951,23 @@ namespace ranges {

// clang-format off
template <input_range _Vw>
requires (view<_Vw> && input_range<range_reference_t<_Vw>>
&& (is_reference_v<range_reference_t<_Vw>> || view<range_value_t<_Vw>>))
requires view<_Vw> && input_range<range_reference_t<_Vw>>
class join_view;
// clang-format on

template <class _Vw>
class _Join_view_base : public view_interface<join_view<_Vw>> {
private:
struct _Cache_wrapper {
template <input_iterator _Iter>
constexpr _Cache_wrapper(_Not_quite_object::_Construct_tag, const _Iter& _It) noexcept(noexcept(*_It))
: _Val(*_It) {}

remove_cv_t<range_reference_t<_Vw>> _Val;
};

protected:
/* [[no_unique_address]] */ _Defaultabox<views::all_t<range_reference_t<_Vw>>> _Inner{};
/* [[no_unique_address]] */ _Non_propagating_cache<_Cache_wrapper> _Inner{};
};

// clang-format off
Expand All @@ -2890,8 +2976,7 @@ namespace ranges {
class _Join_view_base<_Vw> : public view_interface<join_view<_Vw>> {};

template <input_range _Vw>
requires (view<_Vw> && input_range<range_reference_t<_Vw>>
&& (is_reference_v<range_reference_t<_Vw>> || view<range_value_t<_Vw>>))
requires view<_Vw> && input_range<range_reference_t<_Vw>>
class join_view : public _Join_view_base<_Vw> {
// clang-format on
private:
Expand Down Expand Up @@ -2948,20 +3033,19 @@ namespace ranges {
/* [[no_unique_address]] */ _Defaultabox<_InnerIter> _Inner{}; // per LWG issue unfiled as of 2021-06-14
_Parent_t* _Parent{};

constexpr auto& _Update_inner(_InnerRng<_Const> _Range) {
constexpr auto&& _Update_inner() {
if constexpr (_Deref_is_glvalue) {
return _Range;
return *_Outer;
} else {
_Parent->_Inner = views::all(_STD move(_Range));
return *_Parent->_Inner;
return _Parent->_Inner._Emplace(_Not_quite_object::_Construct_tag{}, _Outer)._Val;
}
}

constexpr void _Satisfy() {
const auto _Last = _RANGES end(_Parent->_Range);
for (; _Outer != _Last; ++_Outer) {
auto& _Tmp = _Update_inner(*_Outer);
_Inner = _RANGES begin(_Tmp);
auto&& _Tmp = _Update_inner();
_Inner = _RANGES begin(_Tmp);
if (*_Inner != _RANGES end(_Tmp)) {
return;
}
Expand All @@ -2979,7 +3063,7 @@ namespace ranges {
if constexpr (_Deref_is_glvalue) {
_Last = _RANGES end(*_Outer);
} else {
_Last = _RANGES end(*_Parent->_Inner);
_Last = _RANGES end((*_Parent->_Inner)._Val);
}
_STL_VERIFY(_Inner && *_Inner != _Last, "cannot dereference join_view end iterator");
}
Expand Down Expand Up @@ -3047,7 +3131,7 @@ namespace ranges {
_Satisfy();
}
} else {
if (++*_Inner == _RANGES end(*_Parent->_Inner)) {
if (++*_Inner == _RANGES end((*_Parent->_Inner)._Val)) {
++_Outer;
_Satisfy();
}
Expand Down
5 changes: 4 additions & 1 deletion stl/inc/xutility
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ concept _Has_member_reference = requires {
typename _Ty::reference;
};

template <class _Ty>
concept _Destructible_object = is_object_v<_Ty> && destructible<_Ty>;

template <class>
struct incrementable_traits {};

Expand Down Expand Up @@ -638,14 +641,14 @@ struct iterator_traits : _Iterator_traits_base<_Ty> {
template <class _Ty>
requires is_object_v<_Ty>
struct iterator_traits<_Ty*> {
// clang-format on
using iterator_concept = contiguous_iterator_tag;
using iterator_category = random_access_iterator_tag;
using value_type = remove_cv_t<_Ty>;
using difference_type = ptrdiff_t;
using pointer = _Ty*;
using reference = _Ty&;
};
// clang-format on

namespace ranges {
namespace _Iter_move {
Expand Down
1 change: 1 addition & 0 deletions stl/inc/yvals_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@
// P2259R1 Repairing Input Range Adaptors And counted_iterator
// (partially implemented)
// P2325R3 Views Should Not Be Required To Be Default Constructible
// P2328R1 join_view Should Join All views Of ranges
// P????R? directory_entry::clear_cache()

// _HAS_CXX20 indirectly controls:
Expand Down
70 changes: 67 additions & 3 deletions tests/std/tests/P0896R4_views_join/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ constexpr bool test_one(Outer&& rng, Expected&& expected) {

// clang-format off
constexpr bool can_test = ranges::viewable_range<Outer>
&& input_range<range_reference_t<Outer>>
&& (deref_is_glvalue || ranges::view<Inner>);
&& input_range<range_reference_t<Outer>>;
// clang-format on

if constexpr (can_test) {
Expand Down Expand Up @@ -463,6 +462,24 @@ void test_move_only_views() {
test_one(move_only_view<bidirectional_iterator_tag, test::Common::yes>{input}, expected_ints);
}

constexpr array<string_view, 5> prvalue_input = {{{}, "Hello "sv, {}, "World!"sv, {}}};

constexpr auto ToVector(const int val) {
return vector{val + 1};
}

constexpr auto ToString(const size_t val) {
return string{prvalue_input[val]};
}

struct Immovable {
Immovable() = default;
Immovable(const Immovable&) = delete;
Immovable(Immovable&&) = delete;
Immovable& operator=(const Immovable&) = delete;
Immovable& operator=(Immovable&&) = delete;
};

int main() {
// Validate views
constexpr string_view expected = "Hello World!"sv;
Expand All @@ -473,6 +490,10 @@ int main() {
static_assert(test_one(sp, expected));
test_one(sp, expected);
}
{ // ...copyable rvalue
static_assert(test_one(array<string_view, 5>{{{}, "Hello "sv, {}, "World!"sv, {}}}, expected));
test_one(array<string_view, 5>{{{}, "Hello "sv, {}, "World!"sv, {}}}, expected);
}
// ... move-only
test_move_only_views();

Expand Down Expand Up @@ -504,8 +525,51 @@ int main() {
assert(ranges::equal(joined, result));
}

{ // P2328 range of prvalue array
static constexpr int result[] = {1, 2, 3, 4, 5};
auto ToArray = [](const int i) { return array<int, 1>{i + 1}; };
assert(ranges::equal(views::iota(0, 5) | views::transform(ToArray) | views::join, result));
static_assert(ranges::equal(views::iota(0, 5) | views::transform(ToArray) | views::join, result));
}

{ // P2328 range of prvalue vector using global function
static constexpr int result[] = {1, 2, 3, 4, 5};
assert(ranges::equal(views::iota(0, 5) | views::transform(ToVector) | views::join, result));
#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-934264
static_assert(ranges::equal(views::iota(0, 5) | views::transform(ToVector) | views::join, result));
#endif // not MSVC
}

{ // P2328 range of prvalue vector using lambda
static constexpr int result[] = {1, 2, 3, 4, 5};
auto ToVectorLambda = [](const int i) { return vector{i + 1}; };
assert(ranges::equal(views::iota(0, 5) | views::transform(ToVectorLambda) | views::join, result));
#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-934264
static_assert(ranges::equal(views::iota(0, 5) | views::transform(ToVectorLambda) | views::join, result));
#endif // not MSVC
}

{ // P2328 range of prvalue string using global function
assert(ranges::equal(views::iota(0u, 5u) | views::transform(ToString) | views::join, expected));
#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-934264
static_assert(ranges::equal(views::iota(0u, 5u) | views::transform(ToString) | views::join, expected));
#endif // not MSVC
}

{ // P2328 range of prvalue string using lambda
auto ToStringLambda = [](const size_t i) { return string{prvalue_input[i]}; };
assert(ranges::equal(views::iota(0u, 5u) | views::transform(ToStringLambda) | views::join, expected));
#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-934264
static_assert(ranges::equal(views::iota(0u, 5u) | views::transform(ToStringLambda) | views::join, expected));
#endif // not MSVC
}

miscco marked this conversation as resolved.
Show resolved Hide resolved
{ // Immovable type
auto ToArrayOfImmovable = [](int) { return array<Immovable, 3>{}; };
assert(ranges::distance(views::iota(0, 2) | views::transform(ToArrayOfImmovable) | views::join) == 6);
static_assert(ranges::distance(views::iota(0, 2) | views::transform(ToArrayOfImmovable) | views::join) == 6);
}

STATIC_ASSERT(instantiation_test());
#endif // TRANSITION, VSO-934264
instantiation_test();
}