-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Description
Expected behavior
Dereferencing a rocksdb::autovector::iterator shouldn't be affected by whether the iterator is top-level const.
Actual behavior
rocksdb::autovector::iterator::operator*() and operator->() are const-overloaded, which doesn't conform to the C++ Standard's iterator requirements.
Steps to reproduce the behavior
Here is a test case that works with std::vector and fails to compile with rocksdb::autovector:
C:\Temp>type meow.cpp
#include <util/autovector.h>
#include <iostream>
#include <type_traits>
#include <vector>
int main() {
#ifdef USE_STD_VECTOR
typedef std::vector<int> VectorType;
std::cout << "Testing std::vector." << std::endl;
#else
typedef rocksdb::autovector<int> VectorType;
std::cout << "Testing rocksdb::autovector." << std::endl;
#endif
VectorType vec;
vec.push_back(10);
vec.push_back(20);
vec.push_back(30);
VectorType::iterator iter = vec.begin();
// NOTE: This is a constant iterator to modifiable elements. It is NOT a const_iterator.
VectorType::iterator const iter_const = vec.begin();
static_assert(std::is_same<decltype(*iter), int&>::value, "decltype(*iter) should be int&");
static_assert(std::is_same<decltype(*iter_const), int&>::value, "decltype(*iter_const) should be int&");
std::cout << "This should be 10: " << *iter << std::endl;
++*iter;
std::cout << "This should be 11: " << *iter << std::endl;
std::cout << "This should be 11: " << *iter_const << std::endl;
++*iter_const;
std::cout << "This should be 12: " << *iter_const << std::endl;
}
C:\Temp>cl /EHsc /nologo /W4 /I StephanTLavavej\rocksdb /DUSE_STD_VECTOR meow.cpp && meow
meow.cpp
Testing std::vector.
This should be 10: 10
This should be 11: 11
This should be 11: 11
This should be 12: 12
C:\Temp>cl /EHsc /nologo /W4 /I StephanTLavavej\rocksdb meow.cpp && meow
meow.cpp
meow.cpp(26): error C2338: decltype(*iter_const) should be int&
meow.cpp(33): error C3892: 'iter_const': you cannot assign to a variable that is const
Here is the relevant code:
Lines 123 to 141 in 5b9233b
| reference operator*() { | |
| assert(vect_->size() >= index_); | |
| return (*vect_)[index_]; | |
| } | |
| const_reference operator*() const { | |
| assert(vect_->size() >= index_); | |
| return (*vect_)[index_]; | |
| } | |
| pointer operator->() { | |
| assert(vect_->size() >= index_); | |
| return &(*vect_)[index_]; | |
| } | |
| const_pointer operator->() const { | |
| assert(vect_->size() >= index_); | |
| return &(*vect_)[index_]; | |
| } |
The wording in the C++ Working Draft specifying that an iterator's operator*, operator->, and operator[] shouldn't be const-overloaded is N4835 23.3.5 [iterator.cpp17]/1, 23.3.5.2 [input.iterators], 23.3.5.4 [forward.iterators], and 23.3.5.6 [random.access.iterators].
(This is because iterators are a generalization of pointers, and pointers have shallow const semantics: Elem * const applies constness to the pointer, but not the element. In contrast, containers own their elements, so they have deep const semantics and must const-overload their operator[].)