From 861d0e555bb6d58e810c4cb308ba64e0dfe9f1c0 Mon Sep 17 00:00:00 2001 From: Hui Date: Mon, 5 Jun 2023 14:42:21 +0100 Subject: [PATCH] address LWG feedback on concat, Part 1 --- concat.lwg.feedback.md | 201 ++++++++++++++++++++++++++++++++++++ concat.md | 120 +++++++-------------- impl/concat/concat.hpp | 19 ++-- impl/concat/test/basics.cpp | 10 ++ 4 files changed, 258 insertions(+), 92 deletions(-) create mode 100644 concat.lwg.feedback.md diff --git a/concat.lwg.feedback.md b/concat.lwg.feedback.md new file mode 100644 index 0000000..8594794 --- /dev/null +++ b/concat.lwg.feedback.md @@ -0,0 +1,201 @@ +# Done + +[Done] 5.2 needs to go +[Done] we don't need 24.7.?.1 p2.3 and strick "if this expression is well formed" from 2.2 +[Done] in 24.7.?.2 add exposition-only for these concepts +[Done] we need to require the entire expression to be equality preserving: we could add CC's code +[Done] on p3 the pack needs to be expanded as I put into the chat +[Done] remove the requires from the default constructor an remove the initialization of views_ +[Done] in the size the make_unsigned_t should be make_unsigned_like_t as it could be integer-like +[Done] on size() the {} need to be () +[Done] also CT(0) isn't needed - the sequence is always non-empty +[Done] value_type could use concat_value_t +[Done] we don't need the friends +[Done] the requires on the default ctor isn't needed +[Done] and remove initialization from base-iter() +[Done] op-- is missing a ; +[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" + +# Todos + +[TODO] mattermost: CC: we should ask the aurthors to remove the section numbers +[TODO] mattermost: TZ/CC: wording the concat-bidirectional + +[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 +[TODO] that "expect" needs to be "except" +[TODO] for the op++ and the others we should use the random-access from zip_view +[TODO] the concepts from zip_view [rang.zip.iterator] need to move to a different section: move to [range.utility.helper] +[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 +[TODO] we don't need the preconditions: they come from the "Equivalent to:" => strike p13 and p15 +[TODO] you need p17 but you can rid of p19 +[TODO] we should spell out what reference is in 2.1 +[TODO] put the front matter after the iterator_category +[TODO] we should use a single definition for the relational operators similar to basic_const_iterator [const.iterator.ops] p19 +[TODO] I think the authors need to think about iter_swap and provide rational + +# Full transcript + +JG: this is for C++26 +JG presenting +TK: do we still use tie? +TS: this paper doesn't actually use tuple or pair +TS: 5.2 is already done +JG: this came in with zip or so. +TS: [action] 5.2 needs to go +TK: what about concat of no views? +JG: don't do that +TS: ill-formed +CC: [action] we don't need 24.7.?.1 p2.3 and strick "if this expression is well formed" from 2.2 +TK: do we want to allow concat_view of one view +RD: the pre-matter does talk about 0 and 1 view and it is all_view +TK: but I can still use concat_view explicitly with 1 object +TS: that doesn't seem justified: you can get a take_view of a string_view and get a string_view back +TK: do we have common_reference_t with single argument? +TS: I'm pretty sure we do: we can have it with any number of argument > 0 +TS: we should change the example to use print instead of cout +JG: is there something special about concat-random-access which we don't have under some other concept? +CC: in general these concepts are models requirements and are needed when implementing the view +TK: [action] 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 +RD: do we have a recommended way to word the concepts like the concept-indriectly-readable +RD: how consistent do we want these wordings to be be? +RD: the approach used here is otherwise only by iota_view +RD: there is "exposition-only" repeated for iota_view +JG: [action] in 24.7.?.2 add exposition-only for these concepts +JG: we tend to remove the section names +TS: the concept uses static_cast - where do we require the implicit conversion work? +TK: we required common_refernce_t but not common_reference_with +CC: this is the impl-version; the other requires common_reference_with +TS: we require each of the iterator types to have something convertible_to but I don't see we check that +JG: so we have a required change +TS: it should require convertible_to in addition to the two expressions +TS: if you do things multiple times you end up getting totally nonsensical result +TK: do you mean implicate conversions? +CC: do we change to the requires expression to what I typed in the chat? +TS: [action] we need to require the entire expression to be equality preserving: we could add CC's code +TK: I have another variation how to spell that in the chat +TS: we can do that, too - I don't mind how we spell it +TK: I believe it is the same thing +RD: [action] on p3 the pack needs to be expanded as I put into the chat +TS: the types model a concept; this one is ugly +TS: it needs to say both satisfies and models +JG: why the requirement on 3.2? +CC: we should add a "The" at the begining of 3.1 +CC: [action] that "expect" needs to be "except" +TS: I'd like to say this in code instead of word - or pseudo code +JG: TS's homework is to put suggested wording into the chat or send it to the author +TK: for the end: the last element is always the last element of the pack and then we can apply constness when needed (see chat) +TS: on the concept view: we don't need the default_initializable +JG: [action] remove the requires from the default constructor an remove the initialization of views_ +TK: [action] in the size the make_unsigned_t should be make_unsigned_like_t as it could be integer-like +RD: the as_const stuff is just missing an article in multiple places +CC: [action] on size() the {} need to be () +TS: [action] also CT(0) isn't needed - the sequence is always non-empty +TK: [action] value_type could use concat_value_t +TS: the maybe-const needs to be there +JG: unless we restructure as TK has mentioned +TK: [action] we don't need the friends +JG: do we need the = base-iter();? +TK: it is a variant: it will construct the first element +TK: [action] the requires on the default ctor isn't needed +TK: [action] and remove initialization from base-iter() +TS: it is weird that the default is tied to the first iterator in the range specifically +DK: the default ctor doesn't mean anything +TS: yes, it can only be compared to itself +DK: so we could have a special variant member +TS: it could use monostate instead of depending on the first iterator +TS: I would add monostate to the end and initialize with that +TK: but why at the end? +TS: I think it is nicer at the end if we add monostate +TK: I'd be more comfortable that it is default constructible if any iterator is default constructable +TS: that is harder to implement; it is simple to add monostate and always use that +TS: but then everything visiting the variant is harder - so I take that back: I think I can accept the weirdness +RD: [action] op-- is missing a ; +TK: [action] for the op++ and the others we should use the random-access from zip_view +TS: [action] the concepts from zip_view [rang.zip.iterator] need to move to a different section: move to [range.utility.helper] +TS: maybe we should promote [range.utility.helper] to something higher +CC: seems odd that we have compile-time and run-time range helper; this is a change which can be done outside this paper +TK: all the relational operators should use the random-access +JG: [action] the things which use the general concepts are replaced: op++, op<, etc. +CC: [action] there is iterator const& which should be const iterator& (this is editorial) +CC: [action] (1.3) use all_forward +TK: I thin we can remove parenthesis (2.2.1) around derived from and use binary version of ... +TS: but then you need them at the end; I don't think is helpful +TK: (2.2.3) needs to check forward_iterator +TS: we are just making a C++17 forward into a C++20 input iterator - this is going to be pervasive +TS: having the category lie within the type it isn't that bad +TK: I would still prefer that not to lie if we can avoid it +TS: this one only lies if the iterator lies to us +TS: [action] 2.1 should say is_reference_v +TS: [action] all the other bullets need to start with "otherwise" +TK: I'd prefer using N < (sizeof...(Views) +TS: you'd still need the -1 +TK: [action] OK, we can keep it as is but I prefer < +RD: I put some formatting changes to the chat +TS: on prev(): the block is cartesian-common-arg and use that here: we could get rid of the constexpr +TS: [action] instead of ranges::size this should be ranges::distance and we don't need the static_cast +TS: [action] all the steps need to cast to that iterator's difference_type +TK: if we need to skip multiple ranges and we are emplacing all the iterators and this is observable +TS: if we skip a range entirely we just need to call distance +TS: [issue] do we want to avoid emplacing ranges we don't need to use? +TK: this applies to both advance forward and backward +TS: for advance backward we can also use the common logic +CC: the static_cast are not needed +TS: [action] you should use ranges::distance and then you don't need them +TS: [action] al the steps need to be cast to range::diffence_type +TK: the requirements for random access is too strict: we don't need sized for the last range +TK: we could use the same macro as for bidirectional +SLY: what TK brought we considered; I think that was discussed: there was a reason why we didn't go for that but I can't remember right now +TK: we could look at op- between iterator which is related: this one works +TK: the one with the sentinel need to require sized sentinel +SLY: we are still over constrained for some reason +TS: we don't need sized sentinel: we can get (it - begin) +JG: [action] explain why this is over-constrained +TK: if we think the last range is special for bidirectional it should be consistently special +DK: can we figure that out off-line? +JG: we will certainly need to see this paper again but let's go through all sections +CC: [action] the use of maybe-const on the requirement of the constructor: I think we removed them elsewhere as we Const is true +JG: did we have this in the synopsis as well? +TK: [action] for op*() p10 can use concat_reference_t +TS: [action] we don't need the preconditions: they come from the "Equivalent to:" => strike p13 and p15 +DK: also p17 and p19? +TS: [action] you need p17 but you can rid of p19 +RD: where does "reference" come from? +TS: [action] we should spell out what reference is in 2.1 +TK: in the op[] I'd prefer that we get an iterator i and then dereference *i and we don't say how we get it +JG: are the precondition needed here? +DK: I think when we have "Equivalent To" and it says on the equivalent things we can drop them +CC: I'd prefer the precondition on the op== +TS: can we drop the precondition and rather say in the front-matter say "if valueless_by_acception() is true the iterator is singular" +CC: it isn't directly clear to me that I can't operate on singular iterators and they kind of override the generic requirements +CC: we can do it at least with front matter for this iterator +TS: if you think singular is insufficient, we can say that all operations on singular iterators for concat iterator undefined +TS: [action] put the front matter after the iterator_category +TK: [action] we should use a single definition for the relational operators similar to basic_const_iterator +[const.iterator.ops] p19 +TK: I'll provide a better link +TK: op- for default_sentinel/iterator looks odd +TK: I would prefer to use this is the primary and the other direction negative; it isn't wrong though +CC: iterator const& needs to be fixed to become const iterator& +TK: if we factor maybe-const types in the exception specification should be better +TK: do we want to compute the conditional noexcept on all combination? +TS: that is quadratic +CC: the visit is on the same complexity +TK: I'm not sure if iter_swap behaves correctly when the common_reference_t is prvalues +TS: is that an indirectly swappable thing? +CC: this is a question about iter_swap not about this paper +TK: over here we are hiding swapping iterators with different types behind an interface with one type +TK: we should require the reference types to be swappable +CC: that is circular +TS: I think the authors need to think about iter_swap and provide rational diff --git a/concat.md b/concat.md index 4fd12de..49722fd 100644 --- a/concat.md +++ b/concat.md @@ -382,45 +382,6 @@ namespace std::ranges { } ``` -## Range adaptor helpers [range.adaptor.helpers] - -This paper applies the same following changes as in [@P2374R3]. If [@P2374R3] is -merged into the standard, the changes in this section can be dropped. - -New section after Non-propagating cache [range.nonprop.cache]{.sref}. Move the -definitions of `@_tuple-or-pair_@`, `@_tuple-transform_@`, and -`@_tuple-for-each_@` from Class template `zip_view` [range.zip.view]{.sref} to -this section: - -```cpp -namespace std::ranges { - template - using @_tuple-or-pair_@ = @*see below*@; // exposition only - - template - constexpr auto @_tuple-transform_@(F&& f, Tuple&& tuple) { // exposition only - return apply([&](Ts&&... elements) { - return @_tuple-or-pair_@...>( - invoke(f, std::forward(elements))... - ); - }, std::forward(tuple)); - } - - template - constexpr void @_tuple-for-each_@(F&& f, Tuple&& tuple) { // exposition only - apply([&](Ts&&... elements) { - (invoke(f, std::forward(elements)), ...); - }, std::forward(tuple)); - } -} -``` - -Given some pack of types `Ts`, the alias template `@_tuple-or-pair_@` is defined -as follows: - -1. If `sizeof...(Ts)` is `2`, `@_tuple-or-pair_@` denotes `pair`. -2. Otherwise, `@_tuple-or-pair_@` denotes `tuple`. - ## `concat` Add the following subclause to [range.adaptors]{.sref}. @@ -438,8 +399,7 @@ the expression `views::concat(Es...)` is expression-equivalent to - [2.1]{.pnum} `views::all(Es...)` if `Es` is a pack with only one element and `views::all(Es...)` is a well formed expression, -- [2.2]{.pnum} otherwise, `concat_view(Es...)` if this expression is well formed, -- [2.3]{.pnum} otherwise, ill-formed. +- [2.2]{.pnum} otherwise, `concat_view(Es...)` [*Example:* @@ -489,13 +449,13 @@ namespace std::ranges { requires (view && ...) && (sizeof...(Views) > 0) && @_concatable_@ class concat_view : public view_interface> { - tuple @*views_*@ = tuple(); // exposition only + tuple @*views_*@; // exposition only template class @_iterator_@; // exposition only public: - constexpr concat_view() requires(default_initializable&&...) = default; + constexpr concat_view() = default; constexpr explicit concat_view(Views... views); @@ -520,7 +480,7 @@ namespace std::ranges { ```cpp template -concept @*concat-indirectly-readable*@ = @*see below*@; +concept @*concat-indirectly-readable*@ = @*see below*@; // exposition only ``` :::bq @@ -530,13 +490,14 @@ concept @*concat-indirectly-readable*@ = @*see below*@; ```cpp template -concept @*concat-indirectly-readable-impl*@ = requires (const It it){ - static_cast(*it); - static_cast(ranges::iter_move(it)); +concept @*concat-indirectly-readable-impl*@ = // exposition only +requires (const It it) { + { *it } -> convertible_to; + { ranges::iter_move(it) } -> convertible_to; }; template -concept @*concat-indirectly-readable*@ = +concept @*concat-indirectly-readable*@ = // exposition only common_reference_with<@*concat-reference-t*@&&, @*concat-value-t*@&> && common_reference_with<@*concat-reference-t*@&&, @@ -552,7 +513,7 @@ concept @*concat-indirectly-readable*@ = ```cpp template -concept @_concatable_@ = @*see below*@; +concept @_concatable_@ = @*see below*@; // exposition only ``` :::bq @@ -562,7 +523,7 @@ concept is equivalent to: ```cpp template -concept @_concatable_@ = requires { +concept @_concatable_@ = requires { // exposition only typename @*concat-reference-t*@; typename @*concat-value-t*@; typename @*concat-rvalue-reference-t*@; @@ -573,14 +534,14 @@ concept @_concatable_@ = requires { ```cpp template -concept @_concat-bidirectional_@ = @*see below*@; +concept @_concat-bidirectional_@ = @*see below*@; // exposition only ``` :::bq -[3]{.pnum} The pack `Rs...` models `@_concat-bidirectional_@` if, +[3]{.pnum} `@_concat-bidirectional_@` is satisfied and modeled for `Rs...`, if and only if, -- [3.1]{.pnum} Last element of `Rs...` models `bidirectional_range`, +- [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_@`. ::: @@ -603,7 +564,7 @@ constexpr @_iterator_@ begin() const :::bq -[5]{.pnum} *Effects*: Let `@*is-const*@` be `true` for const-qualified overload, +[5]{.pnum} *Effects*: Let `@*is-const*@` be `true` for the const-qualified overload, and `false` otherwise. Equivalent to: ```cpp @@ -621,9 +582,9 @@ constexpr auto end() const requires(range&&...); :::bq -[6]{.pnum} *Effects*: Let `@*is-const*@` be `true` for const-qualified overload, +[6]{.pnum} *Effects*: Let `@*is-const*@` be `true` for the const-qualified overload, and `false` otherwise, and let `@_last-view_@` be the last element of the pack -`const Views...` for const-qualified overload, and the last element of the pack +`const Views...` for the const-qualified overload, and the last element of the pack `Views...` otherwise. Equivalent to: ```cpp @@ -650,8 +611,8 @@ constexpr auto size() const requires(sized_range&&...); ```cpp return apply( [](auto... sizes) { - using CT = make_unsigned_t>; - return (CT{0} + ... + CT{sizes}); + using CT = @*make-unsigned-like-t*@>; + return (CT(sizes) + ...); }, @_tuple-transform_@(ranges::size, @*views_*@)); ``` @@ -670,7 +631,7 @@ namespace std::ranges{ class concat_view::@_iterator_@ { public: - using value_type = common_type_t>...>; + using value_type = @*concat-value-t*@<@_maybe-const_@...>; using difference_type = common_type_t>...>; using iterator_concept = @*see below*@; using iterator_category = @*see below*@; // not always present. @@ -680,10 +641,7 @@ namespace std::ranges{ variant>...>; @_maybe-const_@* @*parent_*@ = nullptr; // exposition only - @*base-iter*@ @*it_*@ = @*base-iter*@(); // exposition only - - friend class @*iterator*@; - friend class concat_view; + @*base-iter*@ @*it_*@; // exposition only template constexpr void @_satisfy_@(); // exposition only @@ -705,8 +663,7 @@ namespace std::ranges{ public: - @_iterator_@() requires(default_initializable>>&&...) = - default; + @_iterator_@() = default; constexpr @_iterator_@(@_iterator_@ i) requires Const && @@ -725,7 +682,7 @@ namespace std::ranges{ requires @_concat-bidirectional_@<@_maybe-const_@...>; constexpr @_iterator_@ operator--(int) - requires @_concat-bidirectional_@<@_maybe-const_@...> + requires @_concat-bidirectional_@<@_maybe-const_@...>; constexpr @_iterator_@& operator+=(difference_type n) requires @_concat-random-access_@<@_maybe-const_@...>; @@ -775,7 +732,7 @@ namespace std::ranges{ friend constexpr difference_type operator-(default_sentinel_t, const @_iterator_@& x) requires @_concat-random-access_@<@_maybe-const_@...>; - friend constexpr decltype(auto) iter_move(iterator const& it) noexcept(@*see below*@); + friend constexpr decltype(auto) iter_move(const iterator& it) noexcept(@*see below*@); friend constexpr void iter_swap(const iterator& x, const iterator& y) noexcept(@*see below*@) requires @*see below*@; @@ -800,18 +757,17 @@ namespace std::ranges{ `(forward_range<@_maybe-const_@>&&...)` is modeled. In that case, `iterator::iterator_category` is defined as follows: -- [2.1]{.pnum} If `is_lvalue_reference` is `false`, then +- [2.1]{.pnum} If `is_reference_v` is `false`, then `iterator_category` denotes `input_iterator_tag` - [2.2]{.pnum} Otherwise, let `Cs` denote the pack of types `iterator_traits>>::iterator_category...`. - [2.2.1]{.pnum} If `(derived_from && ...) && @_concat-random-access_@<@_maybe-const_@...>` is true, `iterator_category` denotes `random_access_iterator_tag`. - - [2.2.2]{.pnum} If + - [2.2.2]{.pnum} Otherwise, if `(derived_from && ...) && @_concat-bidirectional_@<@_maybe-const_@...>` is true, `iterator_category` denotes `bidirectional_iterator_tag`. - - [2.2.3]{.pnum} If `(derived_from && ...)` is true, - `iterator_category` denotes `forward_iterator_tag`. + - [2.2.3]{.pnum} Otherwise, If `(derived_from && ...)` is true, `iterator_category` denotes `forward_iterator_tag`. - [2.2.4]{.pnum} Otherwise, `iterator_category` denotes `input_iterator_tag`. ```cpp @@ -883,7 +839,7 @@ if constexpr (N == sizeof...(Views) - 1) { get(@*it_*@) += steps; } else { @*it_*@.template emplace(ranges::begin(get(@*parent_*@->@*views_*@))); - advance-fwd(0, offset + steps - n_size); + @*advance-fwd*@(0, offset + steps - n_size); } } ``` @@ -906,10 +862,10 @@ if constexpr (N == 0) { if (offset >= steps) { get(@*it_*@) -= steps; } else { - @*it_*@.template emplace(ranges::begin(get(@*parent_*@->views_)) + - ranges::size(get(@*parent_*@->views_))); - advance-bwd( - static_cast(ranges::size(get(@*parent_*@->views_))), + @*it_*@.template emplace(ranges::begin(get(@*parent_*@->@*views_*@)) + + ranges::size(get(@*parent_*@->@*views_*@))); + @*advance-bwd*@( + static_cast(ranges::size(get(@*parent_*@->@*views_*@))), steps - offset); } } @@ -971,7 +927,7 @@ constexpr @_iterator_@& operator++(); [11]{.pnum} *Preconditions*: `@*it_*@.valueless_by_exception()` is `false`. -[12]{.pnum} *Effects*: Let `@*i*@` be `@*it_.index()*@`. Equivalent to: +[12]{.pnum} *Effects*: Let `@*i*@` be `@*it_*@.index()`. Equivalent to: ```cpp ++get<@*i*@>(@*it_*@); @@ -1025,7 +981,7 @@ constexpr @_iterator_@& operator--() [17]{.pnum} *Preconditions*: `@*it_*@.valueless_by_exception()` is `false`. -[18]{.pnum} *Effects*: Let `@*i*@` be `@*it_.index()*@`. Equivalent to: +[18]{.pnum} *Effects*: Let `@*i*@` be `@*it_*@.index()`. Equivalent to: ```cpp @*prev*@<@*i*@>(); @@ -1036,7 +992,7 @@ return *this; ```cpp constexpr @_iterator_@ operator--(int) - requires @_concat-bidirectional_@<@_maybe-const_@...> + requires @_concat-bidirectional_@<@_maybe-const_@...>; ``` :::bq @@ -1062,7 +1018,7 @@ constexpr @_iterator_@& operator+=(difference_type n) [21]{.pnum} *Preconditions*: `@*it_*@.valueless_by_exception()` is `false`. -[22]{.pnum} *Effects*: Let `@*i*@` be `@*it_.index()*@`. Equivalent to: +[22]{.pnum} *Effects*: Let `@*i*@` be `@*it_*@.index()`. Equivalent to: ```cpp if(n > 0) { @@ -1368,7 +1324,7 @@ return -(x - default_sentinel); ::: ```cpp -friend constexpr decltype(auto) iter_move(iterator const& it) noexcept(@*see below*@); +friend constexpr decltype(auto) iter_move(const iterator& it) noexcept(@*see below*@); ``` :::bq @@ -1379,7 +1335,7 @@ friend constexpr decltype(auto) iter_move(iterator const& it) noexcept(@*see bel ```cpp return std::visit( - [](auto const& i) -> + [](const auto& i) -> common_reference_t>...> { return ranges::iter_move(i); }, diff --git a/impl/concat/concat.hpp b/impl/concat/concat.hpp index 4264721..ef7b44c 100644 --- a/impl/concat/concat.hpp +++ b/impl/concat/concat.hpp @@ -30,8 +30,8 @@ using concat_value_t = common_type_t...>; // clang-format off template concept concat_indirectly_readable_impl = requires (const It it){ - static_cast(*it); - static_cast(ranges::iter_move(it)); + { *it } -> convertible_to; + { ranges::iter_move(it) } -> convertible_to; }; template @@ -234,12 +234,12 @@ template requires (view&&...) && (sizeof...(Views) > 0) && xo::concatable class concat_view : public view_interface> { // clang-format on - tuple views_ = tuple(); // exposition only + tuple views_; // exposition only template class iterator : public xo::iter_cat_base_t { public: - using value_type = common_type_t>...>; + using value_type = xo::concat_value_t<__maybe_const...>; using difference_type = common_type_t>...>; using iterator_concept = decltype(xo::iterator_concept_test()); @@ -248,7 +248,7 @@ class concat_view : public view_interface> { using BaseIt = variant>...>; ParentView* parent_ = nullptr; - BaseIt it_ = BaseIt(); + BaseIt it_; friend class iterator; friend class concat_view; @@ -326,8 +326,7 @@ class concat_view : public view_interface> { public: - iterator() requires(default_initializable>>&&...) = - default; + iterator() = default; constexpr iterator(iterator i) requires Const && @@ -542,7 +541,7 @@ class concat_view : public view_interface> { public: - constexpr concat_view() requires(default_initializable&&...) = default; + constexpr concat_view() = default; constexpr explicit concat_view(Views... views) : views_{static_cast(views)...} {} @@ -586,7 +585,7 @@ class concat_view : public view_interface> { return apply( [](auto... sizes) { using CT = make_unsigned_t>; - return (CT{0} + ... + CT{sizes}); + return (CT(sizes) + ...); }, concat_detail::tuple_transform(ranges::size, views_)); } @@ -636,4 +635,4 @@ inline constexpr xo::concat_fn concat; } // namespace std::ranges -#endif \ No newline at end of file +#endif diff --git a/impl/concat/test/basics.cpp b/impl/concat/test/basics.cpp index d41afca..538b432 100644 --- a/impl/concat/test/basics.cpp +++ b/impl/concat/test/basics.cpp @@ -713,3 +713,13 @@ TEST_POINT("cpp20 input range") { auto x = std::views::concat(r1, r2); [[maybe_unused]] auto it = x.begin(); } + + +TEST_POINT("size") { + std::vector v1 = {1,2}; + auto cv1 = std::ranges::concat_view(std::views::all(v1)); + CHECK(cv1.size() == 2); + + auto cv2 = std::ranges::concat_view(std::views::all(v1), std::views::all(v1)); + CHECK(cv2.size() == 4); +}