[range] Problem with adaptors::map_keys
I'm having some troubles using adaptors::map_keys with custom iterators over Qt associative containers. The problem with Qt associative containers is that they provide iterators over a set of values, not over a set of key-value pairs. I have worked that around by introducing boost range bindings: http://codepad.org/xnZrpZ5i However, when these binding are used, the following code produces a compilation error: BOOST_FOREACH(auto pair, (QMap<int, int>()) | boost::adaptors::map_keys); Everything works fine if the code of boost::range_detail::select_first is slightly modified. You'll find the patch attached. I believe it's a problem in boost range, but I may be wrong, so I need some expert help here. -- Best regards, Alexander Fokin, http://elric.ru.
On 11/28/2010 5:33 AM, Alexander Fokin wrote:
I'm having some troubles using adaptors::map_keys with custom iterators over Qt associative containers.
The problem with Qt associative containers is that they provide iterators over a set of values, not over a set of key-value pairs. I have worked that around by introducing boost range bindings: http://codepad.org/xnZrpZ5i
Three things: - value_type should just be pair< Key, T > (no reference or const qualifications on Key or T) - reference should be pair< Key const &, T& >, with T possibly const-qualified (as you've done) - you can (if you want) use iterator_adaptor instead of iterator_facade to automatically get a correct equal, increment, and decrement.
However, when these binding are used, the following code produces a compilation error:
BOOST_FOREACH(auto pair, (QMap<int, int>()) | boost::adaptors::map_keys);
Everything works fine if the code of boost::range_detail::select_first is slightly modified. You'll find the patch attached.
I believe it's a problem in boost range, but I may be wrong, so I need some expert help here.
There shouldn't be a difference between range_value<R>::type and range_value< R const >::type, so I'd question the correctness of this fix. It might work for you because your value_type's for your mutable iterators and const iterators are different, but like I inferred above, they should be the same. Looking at the code in <boost/range/adaptor/map.hpp>, it looks like some instances of range_value should be replaced with range_reference, e.g., the parameter to select_first< Map >::operator() might be better as range_reference< const Map >::type rather than range_value< Map >::type const &. (Keep in mind I'm looking at version 1.43, so things may have changed since.) You might want to file a trac ticket and see what the maintainer (Neil Groves?) says, or wait and see if others opine on this mailing list... - Jeff
Three things: - value_type should just be pair< Key, T > (no reference or const qualifications on Key or T) - reference should be pair< Key const &, T& >, with T possibly const-qualified (as you've done) - you can (if you want) use iterator_adaptor instead of iterator_facade to automatically get a correct equal, increment, and decrement. I have modified the code as you have suggested and now it compiles OK. The modified code is available at http://codepad.org/AJFIVbdY.
However, there is one thing I'm concerned about. Consider the example at http://codepad.org/plyTCBpx. The following lines do not call the copy constructor of class A: BOOST_FOREACH(auto pair, m) { const A &key = pair.first; } However, when I use range adaptors, the copy constructor is called. That was exactly what I was trying to avoid by using references in value_type. I guess I'll try to fix this by myself and then file a patch. -- Best regards, Alexander Fokin, http://elric.ru.
I have filed a trac ticket: https://svn.boost.org/trac/boost/ticket/4905 -- Best regards, Alexander Fokin, http://elric.ru.
On 11/28/2010 11:44 PM, Alexander Fokin wrote:
Three things: - value_type should just be pair< Key, T> (no reference or const qualifications on Key or T) - reference should be pair< Key const&, T& >, with T possibly const-qualified (as you've done) - you can (if you want) use iterator_adaptor instead of iterator_facade to automatically get a correct equal, increment, and decrement. I have modified the code as you have suggested and now it compiles OK. The modified code is available at http://codepad.org/AJFIVbdY.
However, there is one thing I'm concerned about. Consider the example at http://codepad.org/plyTCBpx. The following lines do not call the copy constructor of class A: BOOST_FOREACH(auto pair, m) { const A&key = pair.first; }
Good.
However, when I use range adaptors, the copy constructor is called. That was exactly what I was trying to avoid by using references in value_type.
It's certainly a reasonable thing to avoid. I'm not sure why A's copy constructor would be called. Perhaps you should run it through a debugger and set a break point in A's copy constructor. I don't have Qt installed so it would take me a little more effort to help you more than that ;) It could be the case that the iterator for the range produced by (m | map_keys) returns by value upon dereferencing, rather than by reference. Might be worth checking...
I guess I'll try to fix this by myself and then file a patch.
Good luck! - Jeff
participants (2)
-
Alexander Fokin -
Jeffrey Lee Hellrung, Jr.