Implement P2325R3 Views Should Not Be Required To Be Default Constructible #2012
Conversation
…ctible"
Major changes:
1. `view` does not refine `default_initializable`. This means that even `span<int, 42>` - which is not default constructible - is a `view`. Replace `_Semiregular_box` with `_Copyable_box` which need not be default constructible when the wrapped type is not.
2. `weakly_incrementable` does not refine `default_initializable`; consequently neither do `input_or_output_iterator`, `input_iterator`, nor `output_iterator`. `forward_iterator` refines `incrementable` refines `regular`, and so is still `default_initalizable`. This proposal reverts the changes from P0896R4 that made `{back_,front_,}insert_iterator` and `ostream{buf,}_iterator` `default_initializable`.
3. The proposal leaves `join_view` broken for ranges of ranges with iterators that aren't default constructible - that defect is fixed here. This will be (but is not yet) the proposed resolution of an LWG issue.
Collateral damage:
* Repurpose `_Semiregular_box` into the working draft's new _`copyable-box`_, which no longer adds default construction behavior. Add a new `optional`-like `_Defaultabox` that augments a `movable` type with default construction behavior for deferred initialization of `view`s and `iterator`s which the working draft depicts as being stored in an `optional`.
* Remove all test cases that were using `span<T, N>` as an exemplar non-`view` `borrowed_range`.
* It's possible for `drop_while_view` and `take_while_view` to have no predicate even when not default-initialized; fixup error message and test cases.
Drive-by: Implement missed `insert_iterator` changes that use `ranges::iterator_t<Container>` instead of `typename Container::iterator`.
CaseyCarter
added
cxx20
stl/inc/iterator
Outdated
| insert_iterator() = default; | ||
| #else // ^^^ __cpp_lib_concepts / !__cpp_lib_concepts vvv | ||
| _CONSTEXPR20 insert_iterator(_Container& _Cont, ranges::iterator_t<_Container> _Where) | ||
| : container(_STD addressof(_Cont)), iter(_STD move(_Where)) {} |
There was a problem hiding this comment.
N4885 [insert.iter.ops]/1 says "Effects: Initializes container with addressof(x) and iter with i." Are we allowed to move here? Should this be restricted to the concepts implementation?
There was a problem hiding this comment.
To me, the intent is that we're allowed to move instead of copy everywhere the standard depicts making a copy of an object that meets the Cpp17CopyConstructible / Cpp17CopyAssignable requirements. Cpp17CopyConstructible / Cpp17CopyAssignable incorporate Cpp17MoveConstructible / Cpp17MoveAssignable, so the move operations are required to be valid and produce "equivalent" results.
That said, the difference is observable. Would you like me to submit an LWG issue to add blanket wording permitting an implementation to weaken copies to moves?
There was a problem hiding this comment.
Yes, I think an LWG issue would be great, thanks. It would need to be carefully phrased since implementations shouldn't be allowed to move from an object multiple times.
In any event, I'm ok with this code as-is.
StephanTLavavej
changed the title
barcharcraz
left a comment
barcharcraz
left a comment
There was a problem hiding this comment.
Only strongly requested change is the ifdef in the test code that may be the wrong direction.
|
|
||
| #if _HAS_CXX23 && defined(__cpp_lib_concepts) // TRANSITION, GH-395 and GH-1814 | ||
| #define __cpp_lib_ranges 201911L | ||
| #define __cpp_lib_ranges 202106L |
There was a problem hiding this comment.
This is tentative right?
There was a problem hiding this comment.
Nope: https://github.com/cplusplus/draft/blob/264e53666f29793a7e38ad8e35fe93b11a82ce17/source/support.tex#L663. The new working draft is WG21-N4892.
|
Thanks for implementing this major C++20 Defect Report - change the past to save the future! 😸 |
4 participants
Major changes:
viewdoes not refinedefault_initializable. This means that evenspan<int, 42>- which is not default constructible - is aview. Replace_Semiregular_boxwith_Copyable_boxwhich need not be default constructible when the wrapped type is not.weakly_incrementabledoes not refinedefault_initializable; consequently neither doinput_or_output_iterator,input_iterator, noroutput_iterator.forward_iteratorrefinesincrementablerefinesregular, and so is stilldefault_initalizable. This proposal reverts the changes from P0896R4 that made{back_,front_,}insert_iteratorandostream{buf,}_iteratordefault_initializable.The proposal leaves
join_viewbroken for ranges of ranges with iterators that aren't default constructible - that defect is fixed here. This will be (but is not yet) the proposed resolution of an LWG issue.The proposal also leaves
basic_stream_viewin a dangerous state by removing the DMI for the stored value: copying the view before the first call tobegincould result in undefined behavior. This will be (but is not yet) the proposed resolution of an LWG issue.This change makes
test::iteratornot default constructible when not modelingforward_iterator, and makestest::rangenot default constructible when the range is immovable or move-only. This gives us some good test coverage for non-default-constructible types.Collateral damage:
_Semiregular_boxinto the working draft's newcopyable-box, which no longer adds default construction behavior. Add a newoptional-like_Defaultaboxthat augments amovabletype with default construction behavior for deferred initialization ofviews anditerators which the working draft depicts as being stored in anoptional.span<T, N>as an exemplar non-viewborrowed_range.drop_while_viewandtake_while_viewto have no predicate even when not default-initialized; fixup error message and test cases.Drive-by: Implement missed
insert_iteratorchanges that useranges::iterator_t<Container>instead oftypename Container::iterator.Fixes #1984.