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.

Sign up for GitHub

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

Jump to bottom

Implement P2325R3 Views Should Not Be Required To Be Default Constructible #2012

Merged
merged 9 commits into from
Jun 29, 2021
Merged

Implement P2325R3 Views Should Not Be Required To Be Default Constructible #2012

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b767943
Implement P2325R3 "Views Should Not Be Required To Be Default Constru…
CaseyCarter Jun 8, 2021
0a74f2b
Annotate proposal number in <yvals_core.h>
CaseyCarter Jun 15, 2021
8cd2340
Update range_alg_support: non-copyable ranges are not default constru…
CaseyCarter Jun 15, 2021
85e054f
Early miscco feedback
CaseyCarter Jun 15, 2021
00eded0
More missco comments
CaseyCarter Jun 15, 2021
208ff0b
Spell `random_access_range` correctly
CaseyCarter Jun 15, 2021
5913610
STL's review comments
CaseyCarter Jun 22, 2021
64f032b
Assignment shouldn't be _NODISCARD.
StephanTLavavej Jun 22, 2021
d4017db
Charlie's review comments
CaseyCarter Jun 26, 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
49 changes: 21 additions & 28 deletions stl/inc/iterator
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ public:

#ifdef __cpp_lib_concepts
using difference_type = ptrdiff_t;

constexpr back_insert_iterator() noexcept = default;
#else // ^^^ __cpp_lib_concepts / !__cpp_lib_concepts vvv
#else
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
using difference_type = void;
#endif // __cpp_lib_concepts

Expand Down Expand Up @@ -64,7 +62,7 @@ public:
}

protected:
_Container* container = nullptr;
_Container* container;
};

// FUNCTION TEMPLATE back_inserter
Expand All @@ -87,9 +85,7 @@ public:

#ifdef __cpp_lib_concepts
using difference_type = ptrdiff_t;

constexpr front_insert_iterator() noexcept = default;
#else // ^^^ __cpp_lib_concepts / !__cpp_lib_concepts vvv
#else
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
using difference_type = void;
#endif // __cpp_lib_concepts

Expand Down Expand Up @@ -119,7 +115,7 @@ public:
}

protected:
_Container* container = nullptr;
_Container* container;
};

// FUNCTION TEMPLATE front_inserter
Expand All @@ -142,13 +138,14 @@ public:
#ifdef __cpp_lib_concepts
using difference_type = ptrdiff_t;

insert_iterator() = default;
#else // ^^^ __cpp_lib_concepts / !__cpp_lib_concepts vvv
_CONSTEXPR20 insert_iterator(_Container& _Cont, ranges::iterator_t<_Container> _Where)
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
: container(_STD addressof(_Cont)), iter(_STD move(_Where)) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

#else // ^^^ implementing Ranges // no Ranges vvv
using difference_type = void;
#endif // __cpp_lib_concepts

_CONSTEXPR20 insert_iterator(_Container& _Cont, typename _Container::iterator _Where)
: container(_STD addressof(_Cont)), iter(_Where) {}
#endif // __cpp_lib_concepts

_CONSTEXPR20 insert_iterator& operator=(const typename _Container::value_type& _Val) {
// insert into container and increment stored iterator
Expand Down Expand Up @@ -176,8 +173,12 @@ public:
}

protected:
_Container* container = nullptr;
typename _Container::iterator iter{};
_Container* container;
#ifdef __cpp_lib_concepts
ranges::iterator_t<_Container> iter;
#else
typename _Container::iterator iter;
#endif // __cpp_lib_concepts
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
};

// FUNCTION TEMPLATE inserter
Expand Down Expand Up @@ -327,10 +328,6 @@ public:
using traits_type = _Traits;
using ostream_type = basic_ostream<_Elem, _Traits>;

#ifdef __cpp_lib_concepts
constexpr ostream_iterator() noexcept = default;
#endif // __cpp_lib_concepts

ostream_iterator(ostream_type& _Ostr, const _Elem* const _Delim = nullptr) noexcept /* strengthened */
: _Mydelim(_Delim), _Myostr(_STD addressof(_Ostr)) {}

Expand All @@ -356,8 +353,8 @@ public:
}

private:
const _Elem* _Mydelim = nullptr; // pointer to delimiter string (NB: not freed)
ostream_type* _Myostr = nullptr; // pointer to output stream
const _Elem* _Mydelim; // pointer to delimiter string (NB: not freed)
ostream_type* _Myostr; // pointer to output stream
};

// CLASS TEMPLATE istreambuf_iterator
Expand Down Expand Up @@ -516,10 +513,6 @@ public:
using streambuf_type = basic_streambuf<_Elem, _Traits>;
using ostream_type = basic_ostream<_Elem, _Traits>;

#ifdef __cpp_lib_concepts
constexpr ostreambuf_iterator() noexcept = default;
#endif // __cpp_lib_concepts

ostreambuf_iterator(streambuf_type* _Sb) noexcept : _Strbuf(_Sb) {}

ostreambuf_iterator(ostream_type& _Ostr) noexcept : _Strbuf(_Ostr.rdbuf()) {}
Expand Down Expand Up @@ -549,8 +542,8 @@ public:
}

private:
bool _Failed = false; // true if any stores have failed
streambuf_type* _Strbuf = nullptr;
bool _Failed = false; // true if any stores have failed
streambuf_type* _Strbuf;
};

#ifdef __cpp_lib_concepts
Expand All @@ -566,7 +559,7 @@ class _Variantish {
public:
constexpr explicit _Variantish(_Common_iterator_construct_tag) noexcept : _Contains{_Variantish_state::_Nothing} {}

constexpr _Variantish() noexcept(is_nothrow_default_constructible_v<_It>)
constexpr _Variantish() noexcept(is_nothrow_default_constructible_v<_It>) requires default_initializable<_It>
: _Iterator{}, _Contains{_Variantish_state::_Holds_iter} {}

template <class... _Types>
Expand Down Expand Up @@ -849,7 +842,7 @@ private:
};

public:
constexpr common_iterator() = default;
constexpr common_iterator() requires default_initializable<_Iter> = default;

constexpr common_iterator(_Iter _Right) noexcept(is_nothrow_move_constructible_v<_Iter>) // strengthened
: _Val{in_place_type<_Iter>, _STD move(_Right)} {}
Expand Down Expand Up @@ -1057,7 +1050,7 @@ public:
using iterator_type = _Iter;

// [counted.iter.const]
constexpr counted_iterator() = default;
constexpr counted_iterator() requires default_initializable<_Iter> = default;
constexpr counted_iterator(_Iter _Right, const iter_difference_t<_Iter> _Diff) noexcept(
is_nothrow_move_constructible_v<_Iter>) // strengthened
: _Current(_STD move(_Right)), _Length(_Diff) {
Expand Down
Loading