diff --git a/concat.lwg.feedback.md b/concat.lwg.feedback.md index 6cd636a..0c0d318 100644 --- a/concat.lwg.feedback.md +++ b/concat.lwg.feedback.md @@ -29,13 +29,46 @@ - [Done] we should spell out what reference is in 2.1 - [Done] mattermost: CC: we should ask the aurthors to remove the section numbers (replaced "27.4" with "?.?") - [Done] (EXTRA TODO) Use std::print instead of std::cout in the example. +- [Done] for the op++ and the others we should use the random-access from zip_view + - Hui: zip_view's op++ requires all-random-access but we need random_access + sized + - Levent: Right. Also, do they mean "op+"? + - Hui: moved `all-random-access` to [range.adaptor.helpers] and reuse +- [Done] the concepts from zip_view [rang.zip.iterator] need to move to a different section: move to [range.utility.helper] + - Hui: I think they refer to all-random-access, all-bidirectional and all-forward. In practice, we can only use all-forward. Not sure if it is worth + - Levent: Agree. Also Casey says "this is a change which can be done outside this paper", anyway. + - Hui: Moved +- [Done] (1.3) use all_forward + - Hui: We could. but it seems to be more effort to reuse this simple concepts than it saves. what do you think? + - Levent: I am mildly for it: It is good for a standard-wide glue. I suggest we walk the extra (micro-)mile. +- [Done] the things which use the general concepts are replaced: op++, op<, etc. + - Hui: Not sure if I understand this. I guess they could either mean + - 1. we could replace `random_access_range>&&...` with `all-random-access`, or + - 2. some of our ops requires `concat-random-access` but others requires `random_access_range>&&...`. We should be consistent? + - Levent: For 2, our use isn't accidental: operator< etc can be implemented + without size. So, for those, we can use `all-random-access`. Which is also + what TK was saying (only the "relational operators"). I think we can also + update `concat-random-access`'s def'n to use `all-random-access` for bonus. -# Todos +- [Done] we should use a single definition for the relational operators similar to basic_const_iterator [const.iterator.ops] p19 + - Hui: I just learned we can specify that way. Not sure if you like it? + - Levent: Love it. (https://eel.is/c++draft/const.iterators.ops#17 right?) + +# Nothing to do +- [Nothing to do] that "expect" needs to be "except" + - Hui: could not find any expect + - Levent: I checked earliest revisions. No "expect" indeed. Not sure what this was. + + +# Random stuff +- Hui: finally there is a compiler that can run our example (sadly only with std::cout) https://godbolt.org/z/e43r5TbKq + +# Todos - [TODO] mattermost: TZ/CC: wording the concat-bidirectional - Hui: From the chat history they don't seem to be happy about our wording and had some alternative wordings in the chat - Levent: Leave this to next round? + - I am bit confused with "Model" vs "Model and Satisfy"... - [TODO] when doing zip_view and cartesian_product we added maybe_const so we don't need to do that everywhere and we should do that here, too - Hui: ? @@ -44,37 +77,13 @@ becoming a standard exposition symbol (https://eel.is/c++draft/ranges in synopsis) after zip and cartesian_product was in. Wants to make sure we don't define it again. (But we don't. So...) - - -- [TODO] that "expect" needs to be "except" - - Hui: could not find any expect - - Levent: I checked earliest revisions. No "expect" indeed. Not sure what this was. - -- [TODO] for the op++ and the others we should use the random-access from zip_view - - Hui: zip_view's op++ requires all-random-access but we need random_access + sized - - Levent: Right. Also, do they mean "op+"? - -- [TODO] the concepts from zip_view [rang.zip.iterator] need to move to a different section: move to [range.utility.helper] - - Hui: I think they refer to all-random-access, all-bidirectional and all-forward. In practice, we can only use all-forward. Not sure if it is worth - - Levent: Agree. Also Casey says "this is a change which can be done outside this paper", anyway. - -- [TODO] the things which use the general concepts are replaced: op++, op<, etc. - - Hui: Not sure if I understand this. I guess they could either mean - - 1. we could replace `random_access_range>&&...` with `all-random-access`, or - - 2. some of our ops requires `concat-random-access` but others requires `random_access_range>&&...`. We should be consistent? - - Levent: For 2, our use isn't accidental: operator< etc can be implemented - without size. So, for those, we can use `all-random-access`. Which is also - what TK was saying (only the "relational operators"). I think we can also - update `concat-random-access`'s def'n to use `all-random-access` for bonus. - -- [TODO] (1.3) use all_forward - - Hui: We could. but it seems to be more effort to reuse this simple concepts than it saves. what do you think? - - Levent: I am mildly for it: It is good for a standard-wide glue. I suggest we walk the extra (micro-)mile. + - Hui: Maybe they meant that we should add `Const` as template parameter for concepts like `concat-random-access` and `concat-bidirectional`, like what they did for `all-random-access` etc. So that we don't repeat the `maybe_const` in the every usage of `concat-random-access`? What do you think? - [TODO] do we want to avoid emplacing ranges we don't need to use? - Hui: Good point. I think this refers to `advance_fwd` and `advance_bwd`. Maybe we can do something smarter. - Levent: Oof. Sure, empty subranges. Let the compiler authors do the optim? (not sure how refined these "equivalent to:" snippets should be) + - Hui: at the moment, if we do `i + n`, if `n` is large enough to skip several ranges in between, we currently specify that it needs to be emplaced for every single begin iterators of the ranges in between, which could be skipped. Not sure how we can do this with an elegant way - [TODO] explain why this is over-constrained - Hui: Not sure what this refers to @@ -84,18 +93,19 @@ `concat-bidirectional`. But I think we thought about this and decided that we need the last one sized also. I didn't remember then, and for sure don't remember now :P + - Hui: Indeed I think the last range does not need to be sized with careful implementation. If you look at the implementation (and the spec) for `x - y`, we calculate all the sizes. But the last value is not used. We could avoid it by carefully doing the bound checking, which is a bit more complicated. In addition, in `x - default_sentinel`, we have to use the last range's size if it is not `common_range`. But this operation (`x - default_sentinel`) is not required for `random_access` - [TODO] put the front matter after the iterator_category - Hui: I think this refers to all the preconditions about variant's valueless state. It is good idea but I don't know how to do it. I mean, how to specify what operations are allowed and what are not. Need prio examples. - Levent: We don't have any operations that are allowed. (no?) + - Hui: Well the generated operations like the destructor and the assignment operators should be allowed. (no?) -- [TODO] we should use a single definition for the relational operators similar to basic_const_iterator [const.iterator.ops] p19 - - Hui: I just learned we can specify that way. Not sure if you like it? - - Levent: Love it. (https://eel.is/c++draft/const.iterators.ops#17 right?) - [TODO] I think the authors need to think about iter_swap and provide rational - Hui: sg9? - Levent: Let's just remove it. + - Hui: I thought Tim brought another issue that even the default behaviour (without our specialisation) of `iter_swap` is still problematic for `concat(strings, string_views)`. + https://godbolt.org/z/49esvec96 # Full transcript diff --git a/concat.md b/concat.md index 4567681..2c8a70b 100644 --- a/concat.md +++ b/concat.md @@ -387,6 +387,45 @@ namespace std::ranges { } ``` +## Move exposition-only utilities + +Move `@*all-random-access*@`, `@*all-bidirectional*@`, and `@*all-forward*@` from [range.zip.iterator]{.sref} to [range.adaptor.helpers]{.sref}. + +Add the following to [range.adaptor.helpers]{.sref} + +```diff +namespace std::ranges { +// [...] ++ template ++ concept all-random-access = // exposition only ++ (random_access_range> && ...); ++ template ++ concept all-bidirectional = // exposition only ++ (bidirectional_range> && ...); ++ template ++ concept all-forward = // exposition only ++ (forward_range> && ...); +// [...] +} +``` + +Remove the following from [range.zip.iterator]{.sref} + +```diff +namespace std::ranges { +- template +- concept all-random-access = // exposition only +- (random_access_range> && ...); +- template +- concept all-bidirectional = // exposition only +- (bidirectional_range> && ...); +- template +- concept all-forward = // exposition only +- (forward_range> && ...); +// [...] +} +``` + ## `concat` Add the following subclause to [range.adaptors]{.sref}. @@ -546,8 +585,8 @@ concept @_concat-bidirectional_@ = @*see below*@; // exposition only [3]{.pnum} `@_concat-bidirectional_@` is satisfied and modeled for `Rs...`, if and only if, -- [3.1]{.pnum} The last element of `Rs...` models `bidirectional_range`, -- [3.2]{.pnum} And, all except the last element of `Rs...` model `@_constant-time-reversible_@`. +- [3.1]{.pnum} The last element of `Rs...` models and satisfies `bidirectional_range`, +- [3.2]{.pnum} And, all except the last element of `Rs...` model and satisfy `@_constant-time-reversible_@`. ::: @@ -678,7 +717,7 @@ namespace std::ranges{ constexpr void operator++(int); constexpr @_iterator_@ operator++(int) - requires(forward_range<@_maybe-const_@>&&...); + requires @*all-forward*@; constexpr @_iterator_@& operator--() requires @_concat-bidirectional_@<@_maybe-const_@...>; @@ -701,20 +740,20 @@ namespace std::ranges{ friend constexpr bool operator==(const @_iterator_@& it, default_sentinel_t); friend constexpr bool operator<(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); + requires @*all-random-access*@; friend constexpr bool operator>(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); + requires @*all-random-access*@; friend constexpr bool operator<=(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); + requires @*all-random-access*@; friend constexpr bool operator>=(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); + requires @*all-random-access*@; friend constexpr auto operator<=>(const @_iterator_@& x, const @_iterator_@& y) - requires((random_access_range<@_maybe-const_@> && - three_way_comparable<@_maybe-const_@>)&&...); + requires (@*all-random-access*@ && + (three_way_comparable<@_maybe-const_@> &&...)); friend constexpr @_iterator_@ operator+(const @_iterator_@& it, difference_type n) requires @_concat-random-access_@<@_maybe-const_@...>; @@ -750,13 +789,12 @@ namespace std::ranges{ - [1.2]{.pnum} Otherwise, if `@_concat-bidirectional_@<@_maybe-const_@...>` is modeled, then `iterator_concept` denotes `bidirectional_iterator_tag`. -- [1.3]{.pnum} Otherwise, if - `(forward_range<@_maybe-const_@> && ...)` is modeled, then +- [1.3]{.pnum} Otherwise, if `@*all-forward*@` is modeled, then `iterator_concept` denotes `forward_iterator_tag`. - [1.4]{.pnum} Otherwise, `iterator_concept` denotes `input_iterator_tag`. [2]{.pnum} The member typedef-name `iterator_category` is defined if and only if -`(forward_range<@_maybe-const_@>&&...)` is modeled. In that case, +`@*all-forward*@` is modeled. In that case, `iterator::iterator_category` is defined as follows: - [2.1]{.pnum} If `is_reference_v<@*concat-reference-t*@<@_maybe-const_@...>>` is `false`, then @@ -895,7 +933,7 @@ explicit constexpr @_iterator_@( ```cpp constexpr @_iterator_@(@_iterator_@ i) requires Const && - (convertible_to, iterator_t<@_maybe-const_@>>&&...); + (convertible_to, iterator_t>&&...); ``` :::bq @@ -957,7 +995,7 @@ constexpr void operator++(int); ```cpp constexpr @_iterator_@ operator++(int) - requires(forward_range<@_maybe-const_@>&&...); + requires @*all-forward*@; ``` :::bq @@ -1102,91 +1140,29 @@ return it.@*it_*@.index() == last_idx && ```cpp friend constexpr bool operator<(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); -``` - -:::bq - -[28]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and -`y.@*it_*@.valueless_by_exception()` are each `false`. - -[29]{.pnum} *Effects*: Equivalent to: - -```cpp -return x.@*it_*@ < y.@*it_*@; -``` - -::: - -```cpp + requires @*all-random-access*@; friend constexpr bool operator>(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); -``` - -:::bq - -[30]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and -`y.@*it_*@.valueless_by_exception()` are each `false`. - -[31]{.pnum} *Effects*: Equivalent to: - -```cpp -return y < x; -``` - -::: - -```cpp + requires @*all-random-access*@; friend constexpr bool operator<=(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); -``` - -:::bq - -[32]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and -`y.@*it_*@.valueless_by_exception()` are each `false`. - -[33]{.pnum} *Effects*: Equivalent to: - -```cpp -return !(y < x); -``` - -::: - -```cpp + requires @*all-random-access*@; friend constexpr bool operator>=(const @_iterator_@& x, const @_iterator_@& y) - requires(random_access_range<@_maybe-const_@>&&...); -``` - -:::bq - -[34]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and -`y.@*it_*@.valueless_by_exception()` are each `false`. - -[35]{.pnum} *Effects*: Equivalent to: - -```cpp -return !(x < y); -``` - -::: - -```cpp + requires @*all-random-access*@; friend constexpr auto operator<=>(const @_iterator_@& x, const @_iterator_@& y) - requires((random_access_range<@_maybe-const_@> && - three_way_comparable<@_maybe-const_@>)&&...); + requires (@*all-random-access*@ && + (three_way_comparable<@_maybe-const_@> &&...)); ``` :::bq -[36]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and +[28]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and `y.@*it_*@.valueless_by_exception()` are each `false`. -[37]{.pnum} *Effects*: Equivalent to: +[29]{.pnum} Let `@*op*@` be the operator. + +[30]{.pnum} *Effects*: Equivalent to: ```cpp -return x.@*it_*@ <=> y.@*it_*@; +return x.@*it_*@ @*op*@ y.@*it_*@; ``` ::: @@ -1198,9 +1174,9 @@ friend constexpr @_iterator_@ operator+(const @_iterator_@& it, difference_type :::bq -[38]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. +[31]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. -[39]{.pnum} *Effects*: Equivalent to: +[32]{.pnum} *Effects*: Equivalent to: ```cpp return @_iterator_@{it} += n; @@ -1215,9 +1191,9 @@ friend constexpr @_iterator_@ operator+(difference_type n, const @_iterator_@& i :::bq -[40]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. +[33]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. -[41]{.pnum} *Effects*: Equivalent to: +[34]{.pnum} *Effects*: Equivalent to: ```cpp return it + n; @@ -1232,9 +1208,9 @@ friend constexpr @_iterator_@ operator-(const @_iterator_@& it, difference_type :::bq -[42]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. +[35]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. -[43]{.pnum} *Effects*: Equivalent to: +[36]{.pnum} *Effects*: Equivalent to: ```cpp return @*iterator*@{it} -= n; @@ -1249,13 +1225,13 @@ friend constexpr difference_type operator-(const @_iterator_@& x, const @_iterat :::bq -[44]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and +[37]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` and `y.@*it_*@.valueless_by_exception()` are each `false`. -[45]{.pnum} *Effects*: Let `@*i~x~*@` denote `x.@*it_*@.index()` and `@*i~y~*@` +[38]{.pnum} *Effects*: Let `@*i~x~*@` denote `x.@*it_*@.index()` and `@*i~y~*@` denote `y.@*it_*@.index()` -- [45.1]{.pnum} if `@*i~x~*@ > @*i~y~*@`, let `@*d~y~*@` denote the distance +- [38.1]{.pnum} if `@*i~x~*@ > @*i~y~*@`, let `@*d~y~*@` denote the distance from `get<@*i~y~*@>(y.@*it_*@)` to the end of `get<@*i~y~*@>(y.@*parent_*@.@*views_*@)`, `@*d~x~*@` denote the distance from the begin of `get<@*i~x~*@>(x.@*parent_*@.@*views_*@)` to @@ -1268,13 +1244,13 @@ denote `y.@*it_*@.index()` return static_cast(@*d~y~*@) + s + static_cast(@*d~x~*@); ``` -- [45.2]{.pnum} otherwise, if `@*i~x~*@ < @*i~y~*@`, equivalent to: +- [38.2]{.pnum} otherwise, if `@*i~x~*@ < @*i~y~*@`, equivalent to: ```cpp return -(y - x); ``` -- [45.3]{.pnum} otherwise, equivalent to: +- [38.3]{.pnum} otherwise, equivalent to: ```cpp return static_cast(get<@*i~x~*@>(x.@*it_*@) - get<@*i~y~*@>(y.@*it_*@)); @@ -1289,9 +1265,9 @@ friend constexpr difference_type operator-(const @_iterator_@& x, default_sentin :::bq -[46]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` is `false`. +[39]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` is `false`. -[47]{.pnum} *Effects*: Let `@*i~x~*@` denote `x.@*it_*@.index()`, `@*d~x~*@` +[40]{.pnum} *Effects*: Let `@*i~x~*@` denote `x.@*it_*@.index()`, `@*d~x~*@` denote the distance from `get<@*i~x~*@>(x.@*it_*@)` to the end of `get<@*i~x~*@>(x.@*parent_*@.@*views_*@)`. For every integer `@*i~x~*@ < @*i*@ < sizeof...(Views)`, let `s` denote the sum of the sizes of @@ -1311,9 +1287,9 @@ friend constexpr difference_type operator-(default_sentinel_t, const @_iterator_ :::bq -[48]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` is `false`. +[41]{.pnum} *Preconditions*: `x.@*it_*@.valueless_by_exception()` is `false`. -[49]{.pnum} *Effects*: Equivalent to: +[42]{.pnum} *Effects*: Equivalent to: ```cpp return -(x - default_sentinel); @@ -1327,9 +1303,9 @@ friend constexpr decltype(auto) iter_move(const iterator& it) noexcept(@*see bel :::bq -[50]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. +[43]{.pnum} *Preconditions*: `it.@*it_*@.valueless_by_exception()` is `false`. -[51]{.pnum} *Effects*: Equivalent to: +[44]{.pnum} *Effects*: Equivalent to: ```cpp return std::visit( @@ -1340,7 +1316,7 @@ return std::visit( it.@*it_*@); ``` -[52]{.pnum} *Remarks*: The exception specification is equivalent to: +[45]{.pnum} *Remarks*: The exception specification is equivalent to: ```cpp ((is_nothrow_invocable_v>>...`, `is_nothrow_invocable_v` is true. -[56]{.pnum} *Remarks*: The expression in the requires-clause is `true` if and +[49]{.pnum} *Remarks*: The expression in the requires-clause is `true` if and only if: For every combination of two types `X` and `Y` in the set of all types in the parameter pack `iterator_t<@_maybe-const_@>>...`, `indirectly_swappable` is modelled.