This question already has an answer here:
-
Is casting std::pair<T1, T2> const& to std::pair<T1 const, T2> const& safe?
3 answers
This code demonstrates the problem I'm trying to solve:
#include <map>
class Point
{
public:
float m_x;
float m_y;
};
typedef std::set<Point *> PointSet;
typedef std::set<const Point * const> ConstPointSet;
float GetMinimumRange(const ConstPointSet &pointSet)
{
float minimumRange(0.0f);
// find the smallest distance between any pair of points in the set
return minimumRange;
}
float GetMinimumRangeWrong(const PointSet &pointSet)
{
PointSet::iterator first(pointSet.begin());
Point * point(*first);
point->m_x = 42.0f; // I want to prevent this
return 0.0f;
}
class PointSet_
{
public:
std::set<Point *> m_pointSet;
float GetMinumumRange() const
{
PointSet::iterator first(m_pointSet.begin());
Point * point(*first);
point->m_x = 42.0f; // I want to prevent this
return 0.0f;
}
};
void test()
{
PointSet myPointSet;
// Add some points to my set
// This fails because the compiler states it can't convert from PointSet to ConstPointSet.
//float minimumRange1(GetMinimumRange(myPointSet));
// reinterpret_cast<> is the only cast that works here, const_cast fails with the same
// complaint as the line above generates
ConstPointSet *myConstPointSet(reinterpret_cast<ConstPointSet *>(&myPointSet));
float minimumRange1(GetMinimumRange(*myConstPointSet));
float minimumRange2(GetMinimumRangeWrong(myPointSet));
}
I want to create a routine that takes a PointSet, evaluates the minimum range between any pair of Points in the set, but that it guarantees that it won't modify the PointSet passed to it in any way at all. It can't modify the members of any referenced Point, it can't change the pointers themselves, nor can it add or remove members from the set
The issue is that the compiler correctly views PointSet and ConstPointSet as different types because of the difference of const qualifiers of the inner type, and therefore refuses to cast between them, even though I'm only adding const qualifiers.
I tried creating a class to contain a PointSet, and creating a const member function, but even in there it allows modification to one of the inner Points. At least MSVC will compile that without complaint. I'll confess I was quite surprised about this.
The only way I've found that works is to use a reinterpret_cast<> to convert a pointer to a PointSet to a pointer to a ConstPointSet. The standard does note that reinterpret_cast<> can be used to add const qualifiers, but does that apply in this case?
If not, is there any way to do what I want? I realize that good code discipline can be used to ensure that GetMinimumRange() doesn't modify the passed PointSet, but I'd like to get those const qualifiers in there for two reasons.
They will ensure that if anyone ever modifies GetMinimumRange() they can't cause it to modify the PointSet.
It will allow the compiler to optimize over the call to GetMinimumRange(). In the absence of the const qualifiers, no assumptions can be made at the calling site regarding values that could be cached across the call, thus possibly leading to redundant fetches of data.
There is no straightforward way, because constness does not propagate through pointers. In a const PointSet, it's the pointers themselves that are const, not the objects they point to. And, like you've discovered, const Point * is a different type from Point *, so std::set<const Point *> is a different type from std::set<Point *>.
I don't like the reinterpret_cast of a STL structure. That is scary to me. STL does all kinds of optimizations based on the type of template parameters. std::vector<bool> being an extreme example. You'd think that std::set<T *> and std::set<const T *> would be laid out the same because they are both pointers, but I wouldn't assume so until I read it in the Standard.
If it were a structure I had written myself, and I could easily verify that the cast would work, it would be less scary but still ugly.
You could write a wrapper class that holds a reference to a std::set<Point *> but only allows const access to its pointed-to Points via iterators. If the pointers are guaranteed to be non-null, your iterator can dereference the points directly. I've written it here as a template:
template <typename T>
class PointerSetViewer
{
public:
PointerSetViewer(std::set<T *> const &set) : set(set) {}
struct iterator : public std::iterator<std::forward_iterator_tag, T const>
{
iterator(typename std::set<T *>::const_iterator it) : it(it) {}
T const &operator*() const { return **it; }
T const *operator->() const { return *it; }
iterator &operator++() { ++it; return *this; }
bool operator==(iterator other) { return it == other.it; }
bool operator!=(iterator other) { return it != other.it; }
private:
typename std::set<T *>::const_iterator it;
};
iterator begin() { return iterator(set.cbegin()); }
iterator end() { return iterator(set.cend()); }
private:
std::set<T *> const &set;
};
It's bulky, but it accomplishes your goals without doing anything risky:
float GetMinimumRangeWrong(PointerSetViewer<Point> &pointSet)
{
PointerSetViewer<Point>::iterator first(pointSet.begin());
first->m_x = 42.0f; // does not compile
}
Also if you're using C++11, you can get some nice range-based for loops:
template <typename T>
PointerSetViewer<T> view_set(std::set<T *> const &set) {
return PointerSetViewer<T>(set);
}
for (Point const &p : view_set(myPointSet)) {
// whatever...
}
Baroque? Yes, but if one piece of baroque library code lets you write 100 pieces of beautiful application code with better type checking, it's probably worth it.
Edit: this doesn't work for set. As pointed out in comments, a non-const set is defined to hold const T, so there is actually nothing we can do.
At this stage I don't see a viable solution other than making PointSet_ actually wrap the set properly, i.e. have the set be private and be careful in your public functions.
Here is a solution I came up with; make the set contain a little wrapper which will propagate the const-ness of itself onto the pointer.
I would have thought there would be a pre-existing class that does this, but none of the std smart pointer classes seem to.
#include <iostream>
#include <set>
template<typename T>
struct qualifier_ptr
{
T *operator->() { return ptr; }
T const *operator->() const { return ptr; }
operator T*() { return ptr; }
operator T const*() const { return ptr; }
qualifier_ptr(T *p): ptr(p) {}
private:
T *ptr;
};
struct Point
{
float m_x;
float m_y;
};
struct PointSet
{
typedef std::set< qualifier_ptr<Point> > SetType;
SetType points;
float foo() const
{
//Point *p = *points.begin(); // error
Point const *p = *points.begin(); // OK
return 0;
}
};
int main()
{
PointSet ps;
PointSet const &cps = ps;
ps.foo(); // OK
cps.foo(); // OK
}
I normally don't like to use conversion operators but it seems appropriate here.
As you stated in the comments that the set is built only once per session, I'd suggest just creating the ConstPointerSet by making a copy:
void test()
{
PointSet myPointSet;
// Add some points to my set
ConstPointSet myConstPointSet{ begin(myPointSet), end(myPointSet) };
float minimumRange1(GetMinimumRange(myConstPointSet));
}
Or wrapp it into a function:
ConstPointSet toConst(const PointSet& pSet){
return ConstPointSet{ cbegin(pSet), cend(pSet) };
}
If you don't need the semantics of a set I'd recommend using a std::vector instead, which is much more efficient to copy or traverse.
