Skip to content

Commit 4a9fe64

Browse files
carsonRadtkecarsonRadtke
committed
Harden span and dyn_array iterators
1 parent 3c1fad2 commit 4a9fe64

4 files changed

Lines changed: 160 additions & 33 deletions

File tree

‎include/gsl/dyn_array‎

Lines changed: 90 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434

3535
namespace gsl
3636
{
37+
38+
template <typename T, typename Allocator>
39+
class dyn_array;
40+
3741
namespace details
3842
{
3943
template <typename T, typename Allocator = std::allocator<T>>
@@ -164,19 +168,16 @@ namespace details
164168
template <typename T>
165169
class dyn_array_iterator
166170
{
167-
using size_type = std::size_t;
168-
169171
public:
170172
using difference_type = std::ptrdiff_t;
171-
using value_type = T;
173+
using value_type = std::remove_cv_t<T>;
172174
using pointer = T*;
173175
using reference = T&;
174176
using const_reference = const T&;
175177
using iterator_category = std::random_access_iterator_tag;
176178

177-
#if defined(__cpp_lib_ranges) && (__cpp_lib_ranges >= 201911L)
178-
constexpr dyn_array_iterator() = default;
179-
#endif /* __cpp_lib_ranges >= 201911L */
179+
private:
180+
using size_type = std::size_t;
180181

181182
constexpr dyn_array_iterator(pointer ptr, size_type pos, size_type end_pos)
182183
: _ptr{ptr}, _pos{pos}, _end_pos{end_pos}
@@ -185,22 +186,86 @@ namespace details
185186
Ensures(_pos <= _end_pos);
186187
}
187188

189+
public:
190+
#if defined(__cpp_lib_ranges) && (__cpp_lib_ranges >= 201911L)
191+
constexpr dyn_array_iterator() = default;
192+
#endif /* __cpp_lib_ranges >= 201911L */
193+
194+
template <typename Allocator>
195+
constexpr dyn_array_iterator(dyn_array<std::remove_const_t<T>, Allocator>& arr)
196+
: dyn_array_iterator{arr.data(), 0, arr.size()}
197+
{}
198+
199+
template <typename Allocator, typename U = T,
200+
std::enable_if_t<std::is_const<U>::value, bool> = true>
201+
constexpr dyn_array_iterator(const dyn_array<std::remove_const_t<T>, Allocator>& arr)
202+
: dyn_array_iterator{arr.data(), 0, arr.size()}
203+
{}
204+
205+
template <typename U,
206+
std::enable_if_t<std::is_const<T>::value && !std::is_const<U>::value &&
207+
std::is_same<std::remove_const_t<T>, U>::value,
208+
bool> = true>
209+
constexpr dyn_array_iterator(const dyn_array_iterator<U>& other)
210+
: dyn_array_iterator{other._ptr, other._pos, other._end_pos}
211+
{}
212+
188213
#if defined(_MSC_VER) && defined(__cpp_lib_ranges) && (__cpp_lib_ranges >= 201911L)
189214
constexpr operator pointer() const { return _ptr + gsl::narrow<size_type>(_pos); }
190215
#endif /* defined(_MSC_VER) && __cpp_lib_ranges >= 201911L */
191216

192-
constexpr auto operator==(const dyn_array_iterator& other) const
217+
template <typename U,
218+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
219+
true>
220+
constexpr auto operator==(const dyn_array_iterator<U>& other) const
193221
{
194222
Expects(_ptr == other._ptr);
195223
Expects(_end_pos == other._end_pos);
196224
return _pos == other._pos;
197225
}
198226

199-
constexpr auto operator!=(const dyn_array_iterator& other) const
227+
template <typename U,
228+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
229+
true>
230+
constexpr auto operator!=(const dyn_array_iterator<U>& other) const
200231
{
201232
return !(*this == other);
202233
}
203234

235+
template <typename U,
236+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
237+
true>
238+
constexpr auto operator<(const dyn_array_iterator<U>& other) const
239+
{
240+
Expects(_ptr == other._ptr);
241+
Expects(_end_pos == other._end_pos);
242+
return _pos < other._pos;
243+
}
244+
245+
template <typename U,
246+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
247+
true>
248+
constexpr auto operator>(const dyn_array_iterator<U>& other) const
249+
{
250+
return other < *this;
251+
}
252+
253+
template <typename U,
254+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
255+
true>
256+
constexpr auto operator<=(const dyn_array_iterator<U>& other) const
257+
{
258+
return !(other < *this);
259+
}
260+
261+
template <typename U,
262+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
263+
true>
264+
constexpr auto operator>=(const dyn_array_iterator<U>& other) const
265+
{
266+
return !(*this < other);
267+
}
268+
204269
constexpr auto operator*() const -> reference
205270
{
206271
Expects(_ptr != nullptr);
@@ -258,30 +323,32 @@ namespace details
258323
return dyn_array_iterator{_ptr, gsl::narrow<size_type>(new_pos), _end_pos};
259324
}
260325

326+
friend constexpr auto operator+(difference_type diff, const dyn_array_iterator& other)
327+
{
328+
return other + diff;
329+
}
330+
261331
constexpr auto operator-(difference_type diff) const { return *this + (-diff); }
262332

263-
constexpr auto operator-(const dyn_array_iterator& other) const
333+
template <typename U,
334+
std::enable_if_t<std::is_same<std::remove_cv_t<U>, value_type>::value, bool> =
335+
true>
336+
constexpr auto operator-(const dyn_array_iterator<U>& other) const
264337
{
265338
Expects(_ptr == other._ptr);
266339
Expects(_end_pos == other._end_pos);
267340
return gsl::narrow<difference_type>(_pos) - gsl::narrow<difference_type>(other._pos);
268341
}
269342

270-
constexpr auto operator[](size_type pos) -> reference
271-
{
272-
Expects(_pos + pos < _end_pos);
273-
return _ptr[_pos + pos];
274-
}
275-
276-
constexpr auto operator[](size_type pos) const -> const_reference
277-
{
278-
return const_cast<dyn_array_iterator&>(*this).operator[](pos);
279-
}
343+
constexpr auto operator[](difference_type diff) const -> reference { return *(*this + diff); }
280344

281345
private:
282346
pointer _ptr{};
283347
size_type _pos{};
284348
size_type _end_pos{};
349+
350+
template <typename>
351+
friend class dyn_array_iterator;
285352
};
286353
} // namespace details
287354

@@ -385,8 +452,8 @@ public:
385452
constexpr auto data() { return base::data(); }
386453
constexpr auto data() const -> const T* { return const_cast<dyn_array&>(*this).data(); }
387454

388-
constexpr auto begin() { return iterator{data(), 0, size()}; }
389-
constexpr auto begin() const { return const_iterator{data(), 0, size()}; }
455+
constexpr auto begin() { return iterator{*this}; }
456+
constexpr auto begin() const { return const_iterator{*this}; }
390457
constexpr auto cbegin() const { return begin(); }
391458

392459
constexpr auto rbegin() { return reverse_iterator{end()}; }
@@ -401,8 +468,8 @@ public:
401468
}
402469
#endif /* _MSC_VER */
403470

404-
constexpr auto end() { return iterator{data(), size(), size()}; }
405-
constexpr auto end() const { return const_iterator{data(), size(), size()}; }
471+
constexpr auto end() { return begin() + size(); }
472+
constexpr auto end() const { return begin() + size(); }
406473
constexpr auto cend() const { return end(); }
407474

408475
constexpr auto rend() { return reverse_iterator{begin()}; }

‎include/gsl/span‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@
7575
namespace gsl
7676
{
7777

78+
template <class ElementType, std::size_t Extent>
79+
class span;
80+
7881
// implementation details
7982
namespace details
8083
{
@@ -137,14 +140,21 @@ namespace details
137140
using _Unchecked_type = pointer;
138141
using _Prevent_inheriting_unwrap = span_iterator;
139142
#endif // _MSC_VER
140-
constexpr span_iterator() = default;
141143

144+
private:
142145
constexpr span_iterator(pointer begin, pointer end, pointer current)
143146
: begin_(begin), end_(end), current_(current)
144147
{
145148
Expects(begin_ <= current_ && current <= end_);
146149
}
147150

151+
public:
152+
constexpr span_iterator() = default;
153+
154+
template <size_t Extent>
155+
constexpr span_iterator(const span<Type, Extent> & sp)
156+
: span_iterator{sp.data(), sp.data() + sp.size(), sp.data()} {}
157+
148158
constexpr operator span_iterator<const Type>() const noexcept
149159
{
150160
return {begin_, end_, current_};
@@ -335,10 +345,14 @@ namespace details
335345
}
336346
#endif
337347

348+
private:
338349
pointer begin_ = nullptr;
339350
pointer end_ = nullptr;
340351
pointer current_ = nullptr;
341352

353+
template <class>
354+
friend class span_iterator;
355+
342356
template <typename Ptr>
343357
friend struct std::pointer_traits;
344358
};
@@ -657,17 +671,12 @@ public:
657671
// [span.iter], span iterator support
658672
constexpr iterator begin() const noexcept
659673
{
660-
const auto data = storage_.data();
661-
GSL_SUPPRESS(bounds.1)
662-
return {data, data + size(), data};
674+
return iterator{*this};
663675
}
664676

665677
constexpr iterator end() const noexcept
666678
{
667-
const auto data = storage_.data();
668-
GSL_SUPPRESS(bounds.1)
669-
const auto endData = data + storage_.size();
670-
return {data, endData, endData};
679+
return begin() + size();
671680
}
672681

673682
constexpr reverse_iterator rbegin() const noexcept { return reverse_iterator{end()}; }

‎tests/dyn_array_tests.cpp‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,31 @@
1717
static_assert(sizeof(gsl::dyn_array<int>) == 2 * sizeof(void*),
1818
"gsl::dyn_array (with the default allocator) should be 16 bytes");
1919

20+
using dyn_array_iterator = gsl::dyn_array<int>::iterator;
21+
using dyn_array_const_iterator = gsl::dyn_array<int>::const_iterator;
22+
23+
static_assert(
24+
!std::is_constructible<dyn_array_iterator, int*, std::size_t, std::size_t>::value,
25+
"dyn_array_iterator bounds must not be publicly constructible");
26+
static_assert(std::is_convertible<dyn_array_iterator, dyn_array_const_iterator>::value,
27+
"dyn_array iterators should convert to const iterators");
28+
static_assert(
29+
std::is_same<decltype(std::declval<dyn_array_iterator>() ==
30+
std::declval<dyn_array_const_iterator>()),
31+
bool>::value,
32+
"dyn_array iterators should compare across constness");
33+
static_assert(std::is_same<decltype(std::declval<dyn_array_iterator>() -
34+
std::declval<dyn_array_const_iterator>()),
35+
std::ptrdiff_t>::value,
36+
"dyn_array iterators should subtract across constness");
37+
static_assert(std::is_same<decltype(std::declval<const dyn_array_iterator&>()[0]), int&>::value,
38+
"const dyn_array iterators should preserve pointee mutability");
39+
2040
#if defined(__cpp_lib_concepts) && (__cpp_lib_concepts >= 202002L)
21-
static_assert(std::input_iterator<gsl::dyn_array<int>::iterator>,
22-
"gsl::dyn_array should expose a valid input_iterator");
41+
static_assert(std::random_access_iterator<dyn_array_iterator>,
42+
"gsl::dyn_array should expose a valid random_access_iterator");
43+
static_assert(std::random_access_iterator<dyn_array_const_iterator>,
44+
"const gsl::dyn_array should expose a valid random_access_iterator");
2345
#endif /* __cpp_lib_concepts >= 202002L */
2446

2547
#if defined(__cpp_lib_ranges) && (__cpp_lib_ranges >= 201911L)

‎tests/span_compatibility_tests.cpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,35 @@ static_assert(std::is_same<gsl::span<const int, 3>::const_reference, const int&>
601601
"span<const int, 3>::const_reference should be const int&");
602602

603603
// assertions for span_iterator
604+
template <typename Iterator, typename = void>
605+
struct exposes_span_iterator_state : std::false_type
606+
{};
607+
608+
template <typename Iterator>
609+
struct exposes_span_iterator_state<
610+
Iterator, decltype(static_cast<void>(std::declval<Iterator&>().begin_),
611+
static_cast<void>(std::declval<Iterator&>().end_),
612+
static_cast<void>(std::declval<Iterator&>().current_))> : std::true_type
613+
{};
614+
615+
constexpr bool span_iterator_converts_to_const_iterator()
616+
{
617+
int values[] = {1};
618+
gsl::span<int> view{values};
619+
gsl::span<const int>::iterator converted = view.begin();
620+
return *converted == 1;
621+
}
622+
623+
static_assert(!std::is_constructible<gsl::span<int>::iterator, int*, int*, int*>::value,
624+
"span_iterator bounds must not be publicly constructible");
625+
static_assert(!exposes_span_iterator_state<gsl::span<int>::iterator>::value,
626+
"span_iterator bounds must not be publicly mutable");
627+
static_assert(
628+
std::is_convertible<gsl::span<int>::iterator, gsl::span<const int>::iterator>::value,
629+
"span iterators should convert to const element iterators");
630+
static_assert(span_iterator_converts_to_const_iterator(),
631+
"span iterator conversion should preserve its position");
632+
604633
static_assert(std::is_same<std::iterator_traits<gsl::span<int>::iterator>::pointer, int*>::value,
605634
"span<int>::iterator's pointer should be int*");
606635
static_assert(

0 commit comments

Comments
 (0)