Const wrapper for iterator

Question

Edit: added clarification for why I want this, and updated the code since I don't have any answers yet

I have a C++11 array-like class which (can be) a wrapper around a random-access iterator. Index-based access and .begin()/.end() can just pass through to the iterator, but there's a bit of complication when the object is const:

 template<class Size, typename DataIterator>
 class Storage : public Size {
 DataIterator iterator;
 public:
 Storage(const DataIterator &iterator, const Size &size) : Size(size), iterator(iterator) {}
 
 auto operator[](index_t i)
 -> decltype(iterator[i]) {
 return iterator[i];
 }
 auto operator[](index_t i) const
 -> MakeConst<decltype(iterator[i])> {
 return iterator[i];
 }
 
 DataIterator begin() {return iterator;}
 ConstWrapper<DataIterator> begin() const {return iterator;}
 DataIterator end() {return iterator + this->size();}
 ConstWrapper<DataIterator> end() const {return iterator + this->size();}
 };

If we simply returned DataIterator from the const version of .begin() and .end(), then people holding a Storage const & would (incorrectly!) be able to modify the array through that iterator.

Containers like std::vector have two separate iterators (::iterator and ::const_iterator), but our Storage class only has the read-write one, so we synthesise one using ConstWrapper:

 template <typename Iterator>
 class ConstWrapper {
 Iterator iterator;
 using traits = std::iterator_traits<Iterator>;
 public:
 using difference_type = typename traits::difference_type;
 using value_type = typename traits::value_type;
 using pointer = ConstWrapper;
 using reference = MakeConst<typename traits::reference>;
 using iterator_category = typename traits::iterator_category;
 
 ConstWrapper() {}
 ConstWrapper(const Iterator &iterator) : iterator(iterator) {}
 
 // The problematic cases:
 auto operator[](index_t i) const
 -> MakeConst<decltype(iterator[i])> {
 return iterator[i];
 }
 auto operator*() const
 -> MakeConst<decltype(*iterator)> {
 return *iterator;
 }
 bool operator!= (const ConstWrapper& other) const {
 return iterator != other.iterator;
 }
 /** All the other random-access iterator methods **/
 };
 
 // Specialisation to prevent infinite loops
 template <typename Iterator>
 class ConstWrapper<ConstWrapper<Iterator>> : ConstWrapper<Iterator> {
 public:
 using ConstWrapper<Iterator>::ConstWrapper;
 };

The implementation forwards all the relevant methods, and has a specialisation for ConstWrapper<ConstWrapper<...>> so that it can't wrap itself.

A key part is MakeConst, which turns types into the correct const variant (unlike simply adding const, which has no effect on references):

 // Converts (T & -> T const &), and (T -> const T)
 using MakeConst = typename std::conditional<
 std::is_reference<T>::value,
 typename std::remove_reference<T>::type const &,
 const T
 >::type;

Does this make sense, and is it OK? Is there something else I could be doing which is more readable/efficient/etc.?

Thanks!

• user228914: edit your question to add your whole project here
• cloudfeet: @Aryan Sorry, I don't understand what's missing. (I didn't think the implementation of the Size classes were relevant.)
• ALX23z: Why make a wrapper around an iterator? It doesn't make any sense. Just use the iterator itself.
• user673679: It's hard to tell exactly what you want to do (and why). It would be nice to have an example of usage for const and non-const versions. If you have C++20, then perhaps you can just use std::span instead ( en.cppreference.com/w/cpp/container/span ).
• ALX23z: Ok, I see what you wanted. ConstWrapper will not work for fundamental reasons as it requires an iterator to instantiate it. A mutable non-const iterator. While if your class is a const it won't be able to supply a non-const iterator unless you use a const cast which you shouldn't do. Also the const and non-iterators might be very different types with very different behaviours - say if accessing the class requires to make a locking operation. For const version one could just do a shared_lock while mutable requires unique_lock You shouldn't just assume it is a trivial wrap.

Answer 1

I don't think your specialization is doing what you think it's doing. https://godbolt.org/z/Gfh7sv

You don't want ConstWrapper<ConstWrapper<Iterator>> to inherit from ConstWrapper<Iterator>; that would mean that a CW<CW<I>> "is-a-kind-of" CW<I>, which isn't true. You don't want an inheritance relationship here. What you want is simply for specializations of Storage to not wrap DataIterators that happen to be specializations of ConstWrapper already. The way you do that is with an extra layer of alias indirection:

template<class T> struct maybe_constwrap { using type = ConstWrapper<T>; };
template<class U> struct maybe_constwrap<ConstWrapper<U>> { using type = ConstWrapper<U>; };
template<class Size, typename DataIterator>
class Storage : public Size {
public:
 using iterator = DataIterator;
 using const_iterator = typename maybe_constwrap<DataIterator>::type;
 ~~~
};

Now, if DataIterator is already a ConstWrapper, then both iterator and const_iterator will be literally the same type... which is what you want.

---

Your MakeConst seems to be predicated on the dubious idea that const _Bit_reference is not going to be assignable-to. That's likely to change in C++23. See https://stackoverflow.com/questions/63412623/should-c20-stdrangessort-not-need-to-support-stdvectorbool for some context.

---

You repeat some metaprogramming in the return types of operator* and operator[]. They should just return the type reference which you have already computed above.

reference operator*() const { return *it_; }

Incidentally, I'm changing your data member's name from iterator to it_, because I suspect it is a terrible idea, Concepts-wise, to have a container-ish class type with a member named iterator which member is (A) not public, and (B) not a type.

---

Your operator!= should probably use the hidden friend idiom:

friend bool operator==(ConstWrapper a, ConstWrapper b) { return a.it_ == b.it_; }
friend bool operator!=(ConstWrapper a, ConstWrapper b) { return a.it_ != b.it_; }

In C++20 you could theoretically omit operator!= and provide operator== only. I don't know yet if that's a good idea.

Passing ConstWrapper by value should be fine, because it holds only an iterator, and iterators are cheap to copy.

• cloudfeet: Thanks, this is really great
• cloudfeet: For the MakeConst point: now you point it out, yeah the constness of pointers/references is divorced from the constness of the values, so the fact that it worked for std::vector<bool>::reference in C++11 was itself not correct. So... am I going to need a whole shim-const-reference class, or does it mean MakeConst just isn't possible?