Skip to content

Commit 568db64

Browse files
author
Thomas Weidner
committed
Make operator* and operator-> const in all iterators.
C++ iterators basically model pointers. Hence a const pointer can still be dereferenced to a mutable object (`T * const` *not* `const T*`). This is not true for many iterators in itertools since they only have non-`const` versions of `operator*`. This also violates C++ iterator concepts, see [the github issue](ryanhaining#91). This change basically replaces all non-`const` dereference operators with `const` ones. This was straight forward in most cases: - Some iterators own the data they return (note: that probably violates [`LegacyForwardIterator`](https://en.cppreference.com/w/cpp/named_req/ForwardIterator)). So those data fields were changed to `mutable`. - `GroupBy` advances the group while dereferencing. This does not work when the iterator is constant. Moved the advancing to the constructor and increment operators. This is the only real behavior change, please review carefully.
1 parent add5acc commit 568db64

23 files changed

+72
-68
lines changed

‎batched.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ class iter::impl::Batcher {
132132
&& (done() || !(sub_iter_ != other.sub_iter_));
133133
}
134134

135-
DerefVec<ContainerT>& operator*() {
135+
DerefVec<ContainerT>& operator*() const {
136136
return *batch_;
137137
}
138138

139-
DerefVec<ContainerT>* operator->() {
139+
DerefVec<ContainerT>* operator->() const {
140140
return batch_.get();
141141
}
142142
};

‎chain.hpp

+13-13
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
#ifndef ITER_CHAIN_HPP_
22
#define ITER_CHAIN_HPP_
33

4-
#include "internal/iter_tuples.hpp"
5-
#include "internal/iterator_wrapper.hpp"
6-
#include "internal/iterbase.hpp"
7-
84
#include <array>
95
#include <iterator>
106
#include <optional>
117
#include <tuple>
128
#include <type_traits>
139
#include <utility>
1410

11+
#include "internal/iter_tuples.hpp"
12+
#include "internal/iterator_wrapper.hpp"
13+
#include "internal/iterbase.hpp"
14+
1515
namespace iter {
1616
namespace impl {
1717
template <typename TupType, std::size_t... Is>
@@ -62,12 +62,12 @@ class iter::impl::Chained {
6262
using ArrowType = iterator_arrow<std::tuple_element_t<0, TupTypeT>>;
6363

6464
template <std::size_t Idx>
65-
static DerefType get_and_deref(IterTupType& iters) {
65+
static DerefType get_and_deref(const IterTupType& iters) {
6666
return *std::get<Idx>(iters);
6767
}
6868

6969
template <std::size_t Idx>
70-
static ArrowType get_and_arrow(IterTupType& iters) {
70+
static ArrowType get_and_arrow(const IterTupType& iters) {
7171
return apply_arrow(std::get<Idx>(iters));
7272
}
7373

@@ -82,8 +82,8 @@ class iter::impl::Chained {
8282
return std::get<Idx>(lhs) != std::get<Idx>(rhs);
8383
}
8484

85-
using DerefFunc = DerefType (*)(IterTupType&);
86-
using ArrowFunc = ArrowType (*)(IterTupType&);
85+
using DerefFunc = DerefType (*)(const IterTupType&);
86+
using ArrowFunc = ArrowType (*)(const IterTupType&);
8787
using IncFunc = void (*)(IterTupType&);
8888
using NeqFunc = bool (*)(const IterTupType&, const IterTupType&);
8989

@@ -137,11 +137,11 @@ class iter::impl::Chained {
137137
check_for_end_and_adjust();
138138
}
139139

140-
decltype(auto) operator*() {
140+
decltype(auto) operator*() const {
141141
return IterData::derefers[index_](iters_);
142142
}
143143

144-
decltype(auto) operator-> () {
144+
decltype(auto) operator->() const {
145145
return IterData::arrowers[index_](iters_);
146146
}
147147

@@ -161,7 +161,7 @@ class iter::impl::Chained {
161161
bool operator!=(const Iterator& other) const {
162162
return index_ != other.index_
163163
|| (index_ != sizeof...(Is)
164-
&& IterData::neq_comparers[index_](iters_, other.iters_));
164+
&& IterData::neq_comparers[index_](iters_, other.iters_));
165165
}
166166

167167
bool operator==(const Iterator& other) const {
@@ -278,11 +278,11 @@ class iter::impl::ChainedFromIterable {
278278
return !(*this != other);
279279
}
280280

281-
iterator_deref<iterator_deref<ContainerT>> operator*() {
281+
iterator_deref<iterator_deref<ContainerT>> operator*() const {
282282
return **sub_iter_p_;
283283
}
284284

285-
iterator_arrow<iterator_deref<ContainerT>> operator->() {
285+
iterator_arrow<iterator_deref<ContainerT>> operator->() const {
286286
return apply_arrow(*sub_iter_p_);
287287
}
288288
};

‎chunked.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,11 @@ class iter::impl::Chunker {
104104
&& (done() || !(sub_iter_ != other.sub_iter_));
105105
}
106106

107-
DerefVec<ContainerT>& operator*() {
107+
DerefVec<ContainerT>& operator*() const {
108108
return *chunk_;
109109
}
110110

111-
DerefVec<ContainerT>* operator->() {
111+
DerefVec<ContainerT>* operator->() const {
112112
return chunk_.get();
113113
}
114114
};

‎combinations.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class iter::impl::Combinator {
4343
friend class Iterator;
4444
constexpr static const int COMPLETE = -1;
4545
std::remove_reference_t<ContainerT>* container_p_;
46-
CombIteratorDeref<ContainerT> indices_;
46+
mutable CombIteratorDeref<ContainerT> indices_;
4747
int steps_{};
4848

4949
public:
@@ -73,11 +73,11 @@ class iter::impl::Combinator {
7373
}
7474
}
7575

76-
CombIteratorDeref<ContainerT>& operator*() {
76+
CombIteratorDeref<ContainerT>& operator*() const {
7777
return indices_;
7878
}
7979

80-
CombIteratorDeref<ContainerT>* operator->() {
80+
CombIteratorDeref<ContainerT>* operator->() const {
8181
return &indices_;
8282
}
8383

‎combinations_with_replacement.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class iter::impl::CombinatorWithReplacement {
4343
friend class Iterator;
4444
constexpr static const int COMPLETE = -1;
4545
std::remove_reference_t<ContainerT>* container_p_;
46-
CombIteratorDeref<ContainerT> indices_;
46+
mutable CombIteratorDeref<ContainerT> indices_;
4747
int steps_;
4848

4949
public:
@@ -60,11 +60,11 @@ class iter::impl::CombinatorWithReplacement {
6060
? 0
6161
: COMPLETE} {}
6262

63-
CombIteratorDeref<ContainerT>& operator*() {
63+
CombIteratorDeref<ContainerT>& operator*() const {
6464
return indices_;
6565
}
6666

67-
CombIteratorDeref<ContainerT>* operator->() {
67+
CombIteratorDeref<ContainerT>* operator->() const {
6868
return &indices_;
6969
}
7070

‎compress.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ class iter::impl::Compressed {
7373
skip_failures();
7474
}
7575

76-
iterator_deref<ContainerT> operator*() {
76+
iterator_deref<ContainerT> operator*() const {
7777
return *sub_iter_;
7878
}
7979

80-
iterator_arrow<ContainerT> operator->() {
80+
iterator_arrow<ContainerT> operator->() const {
8181
return apply_arrow(sub_iter_);
8282
}
8383

‎cycle.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ class iter::impl::Cycler {
5252
sub_begin_{sub_iter},
5353
sub_end_{std::move(sub_end)} {}
5454

55-
iterator_deref<ContainerT> operator*() {
55+
iterator_deref<ContainerT> operator*() const {
5656
return *sub_iter_;
5757
}
5858

59-
iterator_arrow<ContainerT> operator->() {
59+
iterator_arrow<ContainerT> operator->() const {
6060
return apply_arrow(sub_iter_);
6161
}
6262

‎dropwhile.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ class iter::impl::Dropper {
7979
sub_end_{std::move(sub_end)},
8080
filter_func_(&filter_func) {}
8181

82-
typename Holder::reference operator*() {
82+
typename Holder::reference operator*() const {
8383
init_if_first_use();
8484
return item_.get();
8585
}
8686

87-
typename Holder::pointer operator->() {
87+
typename Holder::pointer operator->() const {
8888
init_if_first_use();
8989
return item_.get_ptr();
9090
}

‎enumerate.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ class iter::impl::Enumerable {
8585
Iterator(IteratorWrapper<ContainerT>&& sub_iter, Index start)
8686
: sub_iter_{std::move(sub_iter)}, index_{start} {}
8787

88-
IterYield<ContainerT> operator*() {
88+
IterYield<ContainerT> operator*() const {
8989
return {index_, *sub_iter_};
9090
}
9191

92-
ArrowProxy<IterYield<ContainerT>> operator->() {
92+
ArrowProxy<IterYield<ContainerT>> operator->() const {
9393
return {**this};
9494
}
9595

‎filter.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,12 @@ class iter::impl::Filtered {
9393
sub_end_{std::move(sub_end)},
9494
filter_func_(&filter_func) {}
9595

96-
typename Holder::reference operator*() {
96+
typename Holder::reference operator*() const {
9797
init_if_first_use();
9898
return item_.get();
9999
}
100100

101-
typename Holder::pointer operator->() {
101+
typename Holder::pointer operator->() const {
102102
init_if_first_use();
103103
return item_.get_ptr();
104104
}

‎groupby.hpp

+12-9
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class iter::impl::GroupProducer {
6161
IteratorWrapper<ContainerT> sub_end_;
6262
Holder<ContainerT> item_;
6363
KeyFunc* key_func_;
64-
std::optional<KeyGroupPair<ContainerT>> current_key_group_pair_;
64+
mutable std::optional<KeyGroupPair<ContainerT>> current_key_group_pair_;
6565

6666
public:
6767
using iterator_category = std::input_iterator_tag;
@@ -77,14 +77,17 @@ class iter::impl::GroupProducer {
7777
key_func_(&key_func) {
7878
if (sub_iter_ != sub_end_) {
7979
item_.reset(*sub_iter_);
80+
set_key_group_pair();
8081
}
8182
}
8283

8384
Iterator(const Iterator& other)
8485
: sub_iter_{other.sub_iter_},
8586
sub_end_{other.sub_end_},
8687
item_{other.item_},
87-
key_func_{other.key_func_} {}
88+
key_func_{other.key_func_} {
89+
set_key_group_pair();
90+
}
8891

8992
Iterator& operator=(const Iterator& other) {
9093
if (this == &other) {
@@ -95,6 +98,7 @@ class iter::impl::GroupProducer {
9598
item_ = other.item_;
9699
key_func_ = other.key_func_;
97100
current_key_group_pair_.reset();
101+
set_key_group_pair();
98102
return *this;
99103
}
100104

@@ -103,13 +107,11 @@ class iter::impl::GroupProducer {
103107
// NOTE the implicitly generated move constructor would
104108
// be wrong
105109

106-
KeyGroupPair<ContainerT>& operator*() {
107-
set_key_group_pair();
110+
KeyGroupPair<ContainerT>& operator*() const {
108111
return *current_key_group_pair_;
109112
}
110113

111-
KeyGroupPair<ContainerT>* operator->() {
112-
set_key_group_pair();
114+
KeyGroupPair<ContainerT>* operator->() const {
113115
return &*current_key_group_pair_;
114116
}
115117

@@ -118,6 +120,7 @@ class iter::impl::GroupProducer {
118120
set_key_group_pair();
119121
}
120122
current_key_group_pair_.reset();
123+
set_key_group_pair();
121124
return *this;
122125
}
123126

@@ -163,7 +166,7 @@ class iter::impl::GroupProducer {
163166
}
164167

165168
void set_key_group_pair() {
166-
if (!current_key_group_pair_) {
169+
if (!current_key_group_pair_ && item_) {
167170
current_key_group_pair_.emplace(std::invoke(*key_func_, item_.get()),
168171
Group<ContainerT>{*this, next_key()});
169172
}
@@ -251,11 +254,11 @@ class iter::impl::GroupProducer {
251254
return ret;
252255
}
253256

254-
iterator_deref<ContainerT> operator*() {
257+
iterator_deref<ContainerT> operator*() const {
255258
return group_p_->owner_.get();
256259
}
257260

258-
typename Holder<ContainerT>::pointer operator->() {
261+
typename Holder<ContainerT>::pointer operator->() const {
259262
return group_p_->owner_.get_ptr();
260263
}
261264
};

‎internal/iterator_wrapper.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <cassert>
55
#include <functional>
66
#include <variant>
7+
78
#include "iterbase.hpp"
89

910
namespace iter {

‎permutations.hpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class iter::impl::Permuter {
4848
return *lhs < *rhs;
4949
}
5050

51-
Permutable<ContainerT> working_set_;
51+
mutable Permutable<ContainerT> working_set_;
5252
int steps_{};
5353

5454
public:
@@ -72,11 +72,11 @@ class iter::impl::Permuter {
7272
cmp_iters);
7373
}
7474

75-
Permutable<ContainerT>& operator*() {
75+
Permutable<ContainerT>& operator*() const {
7676
return working_set_;
7777
}
7878

79-
Permutable<ContainerT>* operator->() {
79+
Permutable<ContainerT>* operator->() const {
8080
return &working_set_;
8181
}
8282

‎powerset.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ class iter::impl::Powersetter {
8282
return ret;
8383
}
8484

85-
iterator_deref<CombinatorType<ContainerT>> operator*() {
85+
iterator_deref<CombinatorType<ContainerT>> operator*() const {
8686
return *comb_iter_;
8787
}
8888

89-
iterator_arrow<CombinatorType<ContainerT>> operator->() {
89+
iterator_arrow<CombinatorType<ContainerT>> operator->() const {
9090
apply_arrow(comb_iter_);
9191
}
9292

‎product.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,11 @@ class iter::impl::Productor {
144144
return !(*this != other);
145145
}
146146

147-
TupleDeref<TupleTypeT> operator*() {
147+
TupleDeref<TupleTypeT> operator*() const {
148148
return {(*std::get<Is>(iters_))...};
149149
}
150150

151-
auto operator->() -> ArrowProxy<decltype(**this)> {
151+
auto operator->() const -> ArrowProxy<decltype(**this)> {
152152
return {**this};
153153
}
154154
};

‎reversed.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,11 @@ class iter::impl::Reverser {
8787
Iterator(ReverseIteratorWrapper<ContainerT>&& sub_iter)
8888
: sub_iter_{std::move(sub_iter)} {}
8989

90-
reverse_iterator_deref<ContainerT> operator*() {
90+
reverse_iterator_deref<ContainerT> operator*() const {
9191
return *sub_iter_;
9292
}
9393

94-
reverse_iterator_arrow<ContainerT> operator->() {
94+
reverse_iterator_arrow<ContainerT> operator->() const {
9595
return apply_arrow(sub_iter_);
9696
}
9797

0 commit comments

Comments
 (0)