Skip to content

Commit

Permalink
concat difference_type fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@huixie90
huixie90 committed Jun 5, 2023
1 parent 861d0e5 commit 3bbd18b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 33 deletions.
8 changes: 4 additions & 4 deletions concat.lwg.feedback.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
[Done] there is iterator const& which should be const iterator& (this is editorial)
[Done] 2.1 should say is_reference_v
[Done] all the other bullets need to start with "otherwise"
[Done] you should use ranges::distance and then you don't need them
[Done] instead of ranges::size this should be ranges::distance and we don't need the static_cast.
[Done] all the steps need to cast to that iterator's difference_type
[Done] al the steps need to be cast to range::diffence_type

# Todos

Expand All @@ -30,11 +34,7 @@
[TODO] the things which use the general concepts are replaced: op++, op<, etc.
[TODO] (1.3) use all_forward
[TODO] OK, we can keep it as is but I prefer <
[TODO] instead of ranges::size this should be ranges::distance and we don't need the static_cast
[TODO] all the steps need to cast to that iterator's difference_type
[TODO] do we want to avoid emplacing ranges we don't need to use?
[TODO] you should use ranges::distance and then you don't need them
[TODO] al the steps need to be cast to range::diffence_type
[TODO] explain why this is over-constrained
[TODO] the use of maybe-const on the requirement of the constructor: I think we removed them elsewhere as we Const is true
[TODO] for op*() p10 can use concat_reference_t
Expand Down
30 changes: 16 additions & 14 deletions concat.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -831,15 +831,16 @@ constexpr void @_advance-fwd_@(difference_type offset, difference_type steps); /
[5]{.pnum} *Effects*: Equivalent to:

```cpp
using underlying_diff_type = iter_difference_t<variant_alternative_t<N, @*base-iter*@>>;
if constexpr (N == sizeof...(Views) - 1) {
get<N>(@*it_*@) += steps;
get<N>(@*it_*@) += static_cast<underlying_diff_type>(steps);
} else {
auto n_size = ranges::size(get<N>(@*parent_*@->@*views_*@));
auto n_size = ranges::distance(get<N>(@*parent_*@->@*views_*@));
if (offset + steps < static_cast<difference_type>(n_size)) {
get<N>(@*it_*@) += steps;
get<N>(@*it_*@) += static_cast<underlying_diff_type>(steps);
} else {
@*it_*@.template emplace<N + 1>(ranges::begin(get<N + 1>(@*parent_*@->@*views_*@)));
@*advance-fwd*@<N + 1>(0, offset + steps - n_size);
@*advance-fwd*@<N + 1>(0, offset + steps - static_cast<difference_type>(n_size));
}
}
```
Expand All @@ -856,16 +857,17 @@ constexpr void @_advance-bwd_@(difference_type offset, difference_type steps); /
[6]{.pnum} *Effects*: Equivalent to:

```cpp
using underlying_diff_type = iter_difference_t<variant_alternative_t<N, @*base-iter*@>>;
if constexpr (N == 0) {
get<N>(@*it_*@) -= steps;
get<N>(@*it_*@) -= static_cast<underlying_diff_type>(steps);
} else {
if (offset >= steps) {
get<N>(@*it_*@) -= steps;
get<N>(@*it_*@) -= static_cast<underlying_diff_type>(steps);
} else {
@*it_*@.template emplace<N - 1>(ranges::begin(get<N - 1>(@*parent_*@->@*views_*@)) +
ranges::size(get<N - 1>(@*parent_*@->@*views_*@)));
auto prev_size = ranges::distance(get<N - 1>(@*parent_*@->@*views_*@));
@*it_*@.template emplace<N - 1>(ranges::begin(get<N - 1>(@*parent_*@->@*views_*@)) + prev_size);
@*advance-bwd*@<N - 1>(
static_cast<difference_type>(ranges::size(get<N - 1>(@*parent_*@->@*views_*@))),
static_cast<difference_type>(prev_size),
steps - offset);
}
}
Expand Down Expand Up @@ -1022,9 +1024,9 @@ constexpr @_iterator_@& operator+=(difference_type n)

```cpp
if(n > 0) {
@*advance-fwd*@<@*i*@>(get<@*i*@>(@*it_*@) - ranges::begin(get<@*i*@>(@*parent_*@->@*views_*@)), n);
@*advance-fwd*@<@*i*@>(static_cast<difference_type(get<@*i*@>(@*it_*@) - ranges::begin(get<@*i*@>(@*parent_*@->@*views_*@))), n);
} else if (n < 0) {
@*advance-bwd*@<@*i*@>(get<@*i*@>(@*it_*@) - ranges::begin(get<@*i*@>(@*parent_*@->@*views_*@)), -n);
@*advance-bwd*@<@*i*@>(static_cast<difference_type(get<@*i*@>(@*it_*@) - ranges::begin(get<@*i*@>(@*parent_*@->@*views_*@))), -n);
}
return *this;
```
Expand Down Expand Up @@ -1267,7 +1269,7 @@ denote `y.@*it_*@.index()`
equivalent to

```cpp
return @*d~y~*@ + s + @*d~x~*@;
return static_cast<difference_type>(@*d~y~*@) + s + static_cast<difference_type>(@*d~x~*@);
```

- [48.2]{.pnum} otherwise, if `@*i~x~*@ < @*i~y~*@`, equivalent to:
Expand All @@ -1279,7 +1281,7 @@ denote `y.@*it_*@.index()`
- [48.3]{.pnum} otherwise, equivalent to:

```cpp
return get<@*i~x~*@>(x.@*it_*@) - get<@*i~y~*@>(y.@*it_*@);
return static_cast<difference_type>(get<@*i~x~*@>(x.@*it_*@) - get<@*i~y~*@>(y.@*it_*@));
```

:::
Expand All @@ -1301,7 +1303,7 @@ all the ranges `get<@*i*@>(x.@*parent_*@.@*views_*@)` if there is any, and `0`
otherwise, equivalent to

```cpp
return -(@*d~x~*@ + s);
return -(static_cast<difference_type>(@*d~x~*@) + static_cast<difference_type>(s));
```

:::
Expand Down
32 changes: 17 additions & 15 deletions impl/concat/concat.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,31 +287,33 @@ class concat_view : public view_interface<concat_view<Views...>> {

template <std::size_t N>
constexpr void advance_fwd(difference_type current_offset, difference_type steps) {
using underlying_diff_type = std::iter_difference_t<std::variant_alternative_t<N, BaseIt>>;
if constexpr (N == sizeof...(Views) - 1) {
get<N>(it_) += steps;
get<N>(it_) += static_cast<underlying_diff_type>(steps);
} else {
auto n_size = ranges::size(get<N>(parent_->views_));
auto n_size = ranges::distance(get<N>(parent_->views_));
if (current_offset + steps < static_cast<difference_type>(n_size)) {
get<N>(it_) += steps;
get<N>(it_) += static_cast<underlying_diff_type>(steps);
} else {
it_.template emplace<N + 1>(ranges::begin(get<N + 1>(parent_->views_)));
advance_fwd<N + 1>(0, current_offset + steps - n_size);
advance_fwd<N + 1>(0, current_offset + steps - static_cast<difference_type>(n_size));
}
}
}

template <std::size_t N>
constexpr void advance_bwd(difference_type current_offset, difference_type steps) {
using underlying_diff_type = std::iter_difference_t<std::variant_alternative_t<N, BaseIt>>;
if constexpr (N == 0) {
get<N>(it_) -= steps;
get<N>(it_) -= static_cast<underlying_diff_type>(steps);
} else {
if (current_offset >= steps) {
get<N>(it_) -= steps;
get<N>(it_) -= static_cast<underlying_diff_type>(steps);
} else {
it_.template emplace<N - 1>(ranges::begin(get<N - 1>(parent_->views_)) +
ranges::size(get<N - 1>(parent_->views_)));
auto prev_size = ranges::distance(get<N - 1>(parent_->views_));
it_.template emplace<N - 1>(ranges::begin(get<N - 1>(parent_->views_)) + prev_size);
advance_bwd<N - 1>(
static_cast<difference_type>(ranges::size(get<N - 1>(parent_->views_))),
static_cast<difference_type>(prev_size),
steps - current_offset);
}
}
Expand Down Expand Up @@ -389,11 +391,11 @@ class concat_view : public view_interface<concat_view<Views...>> {
requires xo::concat_random_access<__maybe_const<Const, Views>...> {
if (n > 0) {
xo::visit_i(it_, [this, n](auto I, auto&& it) {
this->advance_fwd<I>(it - ranges::begin(get<I>(parent_->views_)), n);
this->advance_fwd<I>(static_cast<difference_type>(it - ranges::begin(get<I>(parent_->views_))), n);
});
} else if (n < 0) {
xo::visit_i(it_, [this, n](auto I, auto&& it) {
this->advance_bwd<I>(it - ranges::begin(get<I>(parent_->views_)), -n);
this->advance_bwd<I>(static_cast<difference_type>(it - ranges::begin(get<I>(parent_->views_))), -n);
});
}
return *this;
Expand Down Expand Up @@ -477,11 +479,11 @@ class concat_view : public view_interface<concat_view<Views...>> {
difference_type(0));

auto y_to_end = xo::visit_i(y.it_, [&](auto I, auto&& it) {
return all_sizes[I] - (it - ranges::begin(get<I>(y.get_parent_views())));
return all_sizes[I] - (static_cast<difference_type>(it - ranges::begin(get<I>(y.get_parent_views()))));
});

auto begin_to_x = xo::visit_i(x.it_, [&](auto I, auto&& it) {
return it - ranges::begin(get<I>(x.get_parent_views()));
return static_cast<difference_type>(it - ranges::begin(get<I>(x.get_parent_views())));
});

return y_to_end + in_between + begin_to_x;
Expand All @@ -490,7 +492,7 @@ class concat_view : public view_interface<concat_view<Views...>> {
return -(y - x);
} else {
return xo::visit_i(x.it_,
[&](auto I, auto&&) { return get<I>(x.it_) - get<I>(y.it_); });
[&](auto I, auto&&) { return static_cast<difference_type>(get<I>(x.it_) - get<I>(y.it_)); });
}
}

Expand All @@ -507,7 +509,7 @@ class concat_view : public view_interface<concat_view<Views...>> {
std::accumulate(all_sizes.begin() + idx + 1, all_sizes.end(), difference_type(0));

auto i_to_idx_end = xo::visit_i(it.it_, [&](auto I, auto&& i) {
return all_sizes[I] - (i - ranges::begin(get<I>(it.get_parent_views())));
return all_sizes[I] - static_cast<difference_type>(i - ranges::begin(get<I>(it.get_parent_views())));
});
return -(i_to_idx_end + to_the_end);
}
Expand Down

0 comments on commit 3bbd18b

Please sign in to comment.