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.

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

Use iterator concept in vector's range constructor #1794

Merged
merged 14 commits into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 54 additions & 15 deletions stl/inc/vector
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,11 @@ public:
if constexpr (_Is_fwd_iter_v<_Iter>) {
const auto _Count = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_UFirst, _ULast)));
_Construct_n(_Count, _STD move(_UFirst), _STD move(_ULast));
#ifdef __cpp_lib_concepts
} else if constexpr (forward_iterator<_Iter>) {
const auto _Count = _Convert_size<size_type>(_To_unsigned_like(_RANGES distance(_UFirst, _ULast)));
_Construct_n(_Count, _STD move(_UFirst), _STD move(_ULast));
#endif // __cpp_lib_concepts
} else {
auto&& _Alproxy = _GET_PROXY_ALLOCATOR(_Alty, _Getal());
_Container_proxy_ptr<_Alty> _Proxy(_Alproxy, _Mypair._Myval2);
Expand Down Expand Up @@ -1041,7 +1046,7 @@ public:

private:
template <class _Iter>
_CONSTEXPR20 void _Insert_range(const_iterator _Where, _Iter _First, _Iter _Last, input_iterator_tag) {
_CONSTEXPR20 void _Insert_input_range(const_iterator _Where, _Iter _First, _Iter _Last) {
// insert input range [_First, _Last) at _Where
if (_First == _Last) {
return; // nothing to do, avoid invalidating iterators
Expand All @@ -1067,10 +1072,18 @@ private:
}

template <class _Iter>
_CONSTEXPR20 void _Insert_range(const_iterator _Where, _Iter _First, _Iter _Last, forward_iterator_tag) {
_CONSTEXPR20 void _Insert_forward_range(const_iterator _Where, _Iter _First, _Iter _Last) {
// insert forward range [_First, _Last) at _Where
const pointer _Whereptr = _Where._Ptr;
const auto _Count = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_First, _Last)));
size_type _Count;
#ifdef __cpp_lib_concepts
if constexpr (forward_iterator<_Iter>) {
_Count = _Convert_size<size_type>(_To_unsigned_like(_RANGES distance(_First, _Last)));
} else
#endif // __cpp_lib_concepts
{
_Count = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_First, _Last)));
}

auto& _Al = _Getal();
auto& _My_data = _Mypair._Myval2;
Expand Down Expand Up @@ -1195,7 +1208,16 @@ public:

_Adl_verify_range(_First, _Last);
const auto _Whereoff = static_cast<size_type>(_Whereptr - _Oldfirst);
_Insert_range(_Where, _Get_unwrapped(_First), _Get_unwrapped(_Last), _Iter_cat_t<_Iter>{});
#ifdef __cpp_lib_concepts
constexpr bool _Is_fwd = _Is_fwd_iter_v<_Iter> || forward_iterator<_Iter>;
#else // ^^^ __cpp_lib_concepts ^^^ / vvv !__cpp_lib_concepts vvv
constexpr bool _Is_fwd = _Is_fwd_iter_v<_Iter>;
#endif // ^^^ !__cpp_lib_concepts ^^^
if constexpr (_Is_fwd) {
_Insert_forward_range(_Where, _Get_unwrapped(_First), _Get_unwrapped(_Last));
} else {
_Insert_input_range(_Where, _Get_unwrapped(_First), _Get_unwrapped(_Last));
}
return _Make_iterator_offset(_Whereoff);
}

Expand Down Expand Up @@ -1250,7 +1272,7 @@ public:

private:
template <class _Iter>
_CONSTEXPR20 void _Assign_range(_Iter _First, _Iter _Last, input_iterator_tag) {
_CONSTEXPR20 void _Assign_input_range(_Iter _First, _Iter _Last) {
// assign input range [_First, _Last)
auto& _My_data = _Mypair._Myval2;
pointer& _Myfirst = _My_data._Myfirst;
Expand Down Expand Up @@ -1281,14 +1303,22 @@ private:
}

template <class _Iter>
_CONSTEXPR20 void _Assign_range(_Iter _First, _Iter _Last, forward_iterator_tag) {
_CONSTEXPR20 void _Assign_forward_range(_Iter _First, _Iter _Last) {
// assign forward range [_First, _Last)
const auto _Newsize = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_First, _Last)));
auto& _Al = _Getal();
auto& _My_data = _Mypair._Myval2;
pointer& _Myfirst = _My_data._Myfirst;
pointer& _Mylast = _My_data._Mylast;
pointer& _Myend = _My_data._Myend;
size_type _Newsize;
#ifdef __cpp_lib_concepts
if constexpr (forward_iterator<_Iter>) {
AdamBucior marked this conversation as resolved.
Show resolved Hide resolved
_Newsize = _Convert_size<size_type>(_To_unsigned_like(_RANGES distance(_First, _Last)));
} else
#endif // __cpp_lib_concepts
{
_Newsize = _Convert_size<size_type>(static_cast<size_t>(_STD distance(_First, _Last)));
}
auto& _Al = _Getal();
auto& _My_data = _Mypair._Myval2;
pointer& _Myfirst = _My_data._Myfirst;
pointer& _Mylast = _My_data._Mylast;
pointer& _Myend = _My_data._Myend;

constexpr bool _Nothrow_construct = conjunction_v<is_nothrow_constructible<_Ty, _Iter_ref_t<_Iter>>,
_Uses_default_construct<_Alloc, _Ty*, _Iter_ref_t<_Iter>>>;
Expand Down Expand Up @@ -1334,11 +1364,20 @@ public:
template <class _Iter, enable_if_t<_Is_iterator_v<_Iter>, int> = 0>
_CONSTEXPR20 void assign(_Iter _First, _Iter _Last) {
_Adl_verify_range(_First, _Last);
_Assign_range(_Get_unwrapped(_First), _Get_unwrapped(_Last), _Iter_cat_t<_Iter>{});
#ifdef __cpp_lib_concepts
constexpr bool _Is_fwd = _Is_fwd_iter_v<_Iter> || forward_iterator<_Iter>;
#else // ^^^ __cpp_lib_concepts ^^^ / vvv !__cpp_lib_concepts vvv
constexpr bool _Is_fwd = _Is_fwd_iter_v<_Iter>;
#endif // ^^^ !__cpp_lib_concepts ^^^
if constexpr (_Is_fwd) {
_Assign_forward_range(_Get_unwrapped(_First), _Get_unwrapped(_Last));
} else {
_Assign_input_range(_Get_unwrapped(_First), _Get_unwrapped(_Last));
}
}

_CONSTEXPR20 void assign(initializer_list<_Ty> _Ilist) {
_Assign_range(_Ilist.begin(), _Ilist.end(), random_access_iterator_tag{});
_Assign_forward_range(_Ilist.begin(), _Ilist.end());
}

public:
Expand All @@ -1364,7 +1403,7 @@ public:
}

_CONSTEXPR20 vector& operator=(initializer_list<_Ty> _Ilist) {
_Assign_range(_Ilist.begin(), _Ilist.end(), random_access_iterator_tag{});
_Assign_forward_range(_Ilist.begin(), _Ilist.end());
return *this;
}

StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
14 changes: 9 additions & 5 deletions stl/inc/xmemory
Original file line number Diff line number Diff line change
Expand Up @@ -961,12 +961,16 @@ _CONSTEXPR20 void _Destroy_range(_NoThrowFwdIt _First, const _NoThrowSentinel _L
}
}

template <class _Size_type>
_NODISCARD constexpr _Size_type _Convert_size(const size_t _Len) noexcept(is_same_v<_Size_type, size_t>) {
// convert size_t to _Size_type, avoiding truncation
if constexpr (!is_same_v<_Size_type, size_t>) {
template <class _Size_type, class _Unsigned_type>
_NODISCARD constexpr _Size_type _Convert_size(const _Unsigned_type _Len) noexcept(
sizeof(_Unsigned_type) <= sizeof(_Size_type)) {
// convert _Unsigned_type to _Size_type, avoiding truncation
_STL_INTERNAL_STATIC_ASSERT(_Unsigned_type(-1) > 0);
_STL_INTERNAL_STATIC_ASSERT(_Size_type(-1) > 0);

if constexpr (sizeof(_Unsigned_type) > sizeof(_Size_type)) {
if (_Len > (numeric_limits<_Size_type>::max)()) {
_Xlength_error("size_t too long for _Size_type");
_Xlength_error("size is too long for _Size_type");
}
}

Expand Down
27 changes: 27 additions & 0 deletions tests/std/tests/P0896R4_views_transform/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,31 @@ constexpr void iterator_instantiation_test() {
iterator_instantiator::call<test_iterator<contiguous_iterator_tag, CanDifference::yes>>();
}

// GH-1709 "Performance issue in handling range iterators in vector constructor"
void test_gh_1709() {
const vector vec{1, 2, 3, 4, 5};
const auto transformed{vec | views::transform([](int i) { return i * 10; })};
const auto b{ranges::begin(transformed)};
const auto e{ranges::end(transformed)};

{
const vector test_construct(b, e);
assert((test_construct == vector{10, 20, 30, 40, 50}));
}

{
vector test_insert{-6, -7};
test_insert.insert(test_insert.end(), b, e);
assert((test_insert == vector{-6, -7, 10, 20, 30, 40, 50}));
}

{
vector test_assign{-8, -9};
test_assign.assign(b, e);
assert((test_assign == vector{10, 20, 30, 40, 50}));
}
}

int main() {
{ // Validate copyable views
constexpr span<const int> s{some_ints};
Expand Down Expand Up @@ -776,4 +801,6 @@ int main() {
assert(*i1 == 'e');
assert(*i2 == 'h');
}

test_gh_1709();
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@

#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__)

template <typename T>
struct always_false : std::false_type {};

int main() {} // COMPILE-ONLY

template <typename Val>
Expand Down Expand Up @@ -82,14 +85,24 @@ class fancy_pointer {
return *this;
}

void operator++(int) = delete; // avoid postincrement
fancy_pointer operator++(int) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
static_assert(always_false<Val>::value, "avoid postincrement");
fancy_pointer result = *this;
++rep;
return result;
}

fancy_pointer& operator--() {
--rep;
return *this;
}

void operator--(int) = delete; // avoid postdecrement
fancy_pointer operator--(int) {
static_assert(always_false<Val>::value, "avoid postdecrement");
fancy_pointer result = *this;
--rep;
return result;
}

#ifdef _WIN64
fancy_pointer& operator+=(int rhs) {
Expand Down Expand Up @@ -278,9 +291,6 @@ namespace std {
};
} // namespace std

template <typename T>
struct always_false : std::false_type {};

template <typename Val>
struct fancy_allocator {
fancy_allocator() = default;
Expand Down