Skip to content

Commit

Permalink
Use iterator concept in vector's range constructor (#1794)
Browse files Browse the repository at this point in the history 
Co-authored-by: Stephan T. Lavavej <[email protected]>
  • Loading branch information
@AdamBucior @StephanTLavavej
AdamBucior and StephanTLavavej committed Jun 12, 2022
1 parent 17fde2c commit 877caba
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 25 deletions.
69 changes: 54 additions & 15 deletions stl/inc/vector
View file
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>) {
_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;
}

Expand Down
14 changes: 9 additions & 5 deletions stl/inc/xmemory
View file
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
View file
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();
}
20 changes: 15 additions & 5 deletions tests/std/tests/VSO_0000000_fancy_pointers/test.compile.pass.cpp
View file
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) {
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

0 comments on commit 877caba

Please sign in to comment.