[foreach] boost_foreach_is_lightweight_proxy doesn't work
Hi, i'm trying to modify rvalue container using Boost.ForEach:
#include <string>
#include <vector>
#include
Raider wrote:
Hi, i'm trying to modify rvalue container using Boost.ForEach:
#include <string> #include <vector> #include
typedef std::vectorstd::string vec;
inline boost::mpl::true_ * boost_foreach_is_lightweight_proxy(vec *&, boost::foreach::tag) { return 0; }
vec GetVector() { vec v; v.push_back("hello"); v.push_back("world"); return v; }
int main() { // compiles ok GetVector().push_back("oh god, it's writable!");
// next line gives "unable to convert `const string' to `string&'" BOOST_FOREACH(std::string& String, GetVector()) String.clear();
return 0; }
It doen't compiles (MCVC 8.0) - it looks like boost_foreach_is_lightweight_proxy is ignored. What's wrong?
It's not being ignored --- std::vector is simply not a lightweight proxy. You're lying to BOOST_FOREACH, and it's found you out. :-) The docs say this about lightweight proxy types:
A pair of iterators is an example of a lightweight proxy. It does not store the values of the sequence; rather, it stores iterators to them. This means that iterating over a copy of the proxy object will give the same results as using the object itself.
Iterating over a copy of a std::vector isn't the same thing as as iterating over the original vector because the elements have been copied. When BOOST_FOREACH has to make a copy of a sequence (and it always copies lightweight proxies), it treats the copy as const because it's a temporary object. That's fine for *real* proxies because the const-ness of the proxy doesn't affect the constness of the referred-to elements. I think BOOST_FOREACH is doing the right thing here. -- Eric Niebler Boost Consulting www.boost-consulting.com The Astoria Seminar ==> http://www.astoriaseminar.com
It doen't compiles (MCVC 8.0) - it looks like boost_foreach_is_lightweight_proxy is ignored. What's wrong?
It's not being ignored --- std::vector is simply not a lightweight proxy. You're lying to BOOST_FOREACH, and it's found you out. :-) The docs say this about lightweight proxy types:
A pair of iterators is an example of a lightweight proxy. It does not store the values of the sequence; rather, it stores iterators to them. This means that iterating over a copy of the proxy object will give the same results as using the object itself.
Iterating over a copy of a std::vector isn't the same thing as as iterating over the original vector because the elements have been copied. When BOOST_FOREACH has to make a copy of a sequence (and it always copies lightweight proxies), it treats the copy as const because it's a temporary object. That's fine for *real* proxies because the const-ness of the proxy doesn't affect the constness of the referred-to elements.
I think BOOST_FOREACH is doing the right thing here.
It's not about std::vector, it's about boost_foreach_is_lightweight_proxy. I used std::vector only to make example code small. If you think std::vector make sence, you can change "typedef std::vectorstd::string vec;" line by the following: typedef vector<string> sv; sv global; class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::const_iterator const_iterator; iterator begin() { return v_.begin(); } iterator end() { return v_.end(); } const_iterator begin() const { return v_.begin(); } const_iterator end() const { return v_.end(); } void push_back(const string& s) { v_.push_back(s); } private: sv& v_; }; It's a *real* _lightweight_ proxy, but still getting a compiler error!
Raider wrote:
It doen't compiles (MCVC 8.0) - it looks like boost_foreach_is_lightweight_proxy is ignored. What's wrong? It's not being ignored --- std::vector is simply not a lightweight proxy. You're lying to BOOST_FOREACH, and it's found you out. :-) The docs say this about lightweight proxy types:
A pair of iterators is an example of a lightweight proxy. It does not store the values of the sequence; rather, it stores iterators to them. This means that iterating over a copy of the proxy object will give the same results as using the object itself. Iterating over a copy of a std::vector isn't the same thing as as iterating over the original vector because the elements have been copied. When BOOST_FOREACH has to make a copy of a sequence (and it always copies lightweight proxies), it treats the copy as const because it's a temporary object. That's fine for *real* proxies because the const-ness of the proxy doesn't affect the constness of the referred-to elements.
I think BOOST_FOREACH is doing the right thing here.
It's not about std::vector, it's about boost_foreach_is_lightweight_proxy. I used std::vector only to make example code small. If you think std::vector make sence, you can change "typedef std::vectorstd::string vec;" line by the following:
typedef vector<string> sv; sv global; class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::const_iterator const_iterator; iterator begin() { return v_.begin(); } iterator end() { return v_.end(); } const_iterator begin() const { return v_.begin(); } const_iterator end() const { return v_.end(); } void push_back(const string& s) { v_.push_back(s); } private: sv& v_; };
It's a *real* _lightweight_ proxy, but still getting a compiler error!
Better, but you have proxied a non-const vector, and you're presenting a const interface. Try this: class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::iterator const_iterator; typedef sv::const_iterator const_iterator; iterator begin() const { return v_.begin(); } iterator end() const { return v_.end(); } void push_back(const string& s) const { v_.push_back(s); } private: sv& v_; }; The const-ness of the proxy itself shouldn't matter. Only the const-ness of the proxied object should matter. HTH, -- Eric Niebler Boost Consulting www.boost-consulting.com The Astoria Seminar ==> http://www.astoriaseminar.com
Eric Niebler wrote:
class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::iterator const_iterator; typedef sv::const_iterator const_iterator;
whoops, delete this line ---^^^
iterator begin() const { return v_.begin(); } iterator end() const { return v_.end(); } void push_back(const string& s) const { v_.push_back(s); } private: sv& v_; };
-- Eric Niebler Boost Consulting www.boost-consulting.com The Astoria Seminar ==> http://www.astoriaseminar.com
Better, but you have proxied a non-const vector, and you're presenting a const interface. Try this:
class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::iterator const_iterator; typedef sv::const_iterator const_iterator; iterator begin() const { return v_.begin(); } iterator end() const { return v_.end(); } void push_back(const string& s) const { v_.push_back(s); } private: sv& v_; };
The const-ness of the proxy itself shouldn't matter. Only the const-ness of the proxied object should matter.
I want const_iterator to be dereferencable to const object and iterator to non-cont object. Like this: class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::const_iterator const_iterator; iterator begin() const { return v_.begin(); } iterator end() const { return v_.end(); } void push_back(const string& s) const { v_.push_back(s); } private: sv& v_; }; This allows me make no difference does GetVector() return `vec' type (proxy) or `sv&' (reference to the real container): for (xxx::iterator it(GetVector().begin(), end(GetVector().end()); it != end; ++it) { modify(*it); } for (xxx::const_iterator it(GetVector().begin(), end(GetVector().end()); it != end; ++it) { /* here I'm SURE that (*it) is READ-ONLY: this knowlege make my code safer because comiler will detect mutations and disallow them */ } I want to use BOOST_FOREACH instead of `for' statements above to: 1. don't care that `xxx' type is 2. call GetVector() only once Now I found some unsteady way: #define BOOST_FOREACH_NO_RVALUE_DETECTION makes my code compile. I'll try to dig in this direction.
Raider wrote:
The const-ness of the proxy itself shouldn't matter. Only the const-ness of the proxied object should matter.
I want const_iterator to be dereferencable to const object and iterator to non-cont object. Like this:
class vec { public: vec(sv& v = global) : v_(v) {} typedef sv::iterator iterator; typedef sv::const_iterator const_iterator; iterator begin() const { return v_.begin(); } iterator end() const { return v_.end(); } void push_back(const string& s) const { v_.push_back(s); } private: sv& v_; };
It's not wrong to want that. But it is for important safety reasons that BOOST_FOREACH treats copies of containers as const. In some of the examples you've shown so far, you're trying to proxy a temporary object so you can modify it. That's never going to work. Like this:
for (xxx::iterator it(GetVector().begin(), end(GetVector().end()); it != end; ++it) { modify(*it); }
That's not even valid C++. But even if it were and GetVector() returns a temporary object or a proxy to a temporary object, the vector will be destroyed before you can iterate it. The interface I chose preserves object lifetimes and prevents inadvertent mutation of temporary objects. Weakening its guarantees is a bad idea. If you want the proxy to offer either a const or mutable interface *and* you want your proxy to work with BOOST_FOREACH, you should base it on the const-ness or mutability of the object being proxied, not the constness of the proxy itself. Consider: template<class Range> struct proxy { typedef typename range_result_iterator<Range>::type iterator; typedef typename range_result_iterator<Range>::type const_iterator; iterator begin() const { return boost::begin(rng_); } iterator end() const { return boost::end(rng_); } // etc ... Range &rng_; }; Now, you can have const and mutable proxied objects like: // ok, a mutable proxy proxy< std::vector<int> > p1; // ok, still a mutable proxy proxy< std::vector<int> > const p2; // ok, a const proxy proxy< std::vector<int> const > p3; // still a const proxy proxy< std::vector<int> const > const p4; You can easily write a helper function to create the proxy object for you that has the correct const behavior.
This allows me make no difference does GetVector() return `vec' type (proxy) or `sv&' (reference to the real container):
As does the above.
Now I found some unsteady way: #define BOOST_FOREACH_NO_RVALUE_DETECTION makes my code compile. I'll try to dig in this direction.
Don't do it. Bad things can happen if you use rvalues after you've turned off foreach's rvalue detection. -- Eric Niebler Boost Consulting www.boost-consulting.com The Astoria Seminar ==> http://www.astoriaseminar.com
It's not wrong to want that. But it is for important safety reasons that BOOST_FOREACH treats copies of containers as const. In some of the examples you've shown so far, you're trying to proxy a temporary object so you can modify it. That's never going to work. Like this:
for (xxx::iterator it(GetVector().begin(), end(GetVector().end()); it != end; ++it) { modify(*it); }
That's not even valid C++. But even if it were and GetVector() returns a temporary object or a proxy to a temporary object, the vector will be destroyed before you can iterate it. The interface I chose preserves object lifetimes and prevents inadvertent mutation of temporary objects. Weakening its guarantees is a bad idea.
Eric, thanks for detail explanation! The several tradeoffs you met, I think you choose the right solution.
participants (2)
-
Eric Niebler -
Raider