On 10 May 2013 12:55, Neil Groves wrote:
On Fri, May 10, 2013 at 11:28 AM, Jonathan Wakely
It looks as though it's undefined behaviour to zip ranges of different lengths, because you'll walk off the end of the shorter ranges.
My variadic zip stops at the end of the shortest range, which seems to
be consistent with zip functions in most other languages I've looked at.
I like being able to avoid the cost of checking for the end of every item in the zip especially for non-random access iterators. In anything I put into Boost.Range I think it of paramount importance to obey the zero overhead principle. It seems that it would be simple to allow both end detection mechanisms.
Hi Neil, That makes sense. My implementation always truncates the ranges to the shortest length but I should make it unchecked and then provide a second interface to do the checking and truncating if needed.
Your adaptors also get dangling references if used with rvalue ranges, although this is a problem with the existing boost range adaptors too.
Yes, this has come up numerous times. It's a problem far beyond just ranges and range adaptors. Knowing you a little, I suspect you have a solution I have not thought of to better deal with the issue.
Is the variadic zip iterator you implemented public?
I don't have my own zip_iterator (well, I do, but I'm still working on it :-) but my zip() function is at https://gitorious.org/redistd/redistd/blobs/master/include/redi/zip.h and just uses boost::zip_iterator. The solution to the dangling reference problem is surprisingly simple in C++11: template<typename Traversable> struct adaptor { explicit adaptor(Traversable&& r) : range(r) { } Traversable range; auto begin() const -> decltype(range.begin()) { return range.begin(); } auto end() const -> decltype(range.end()) { return range.end(); } }; // When called with lvalue, Traversable deduces to R& // When called with rvalue, Traversable deduces to R template<typename Traversable> adaptor<Traversable> adapt(Traversable&& t) { return adaptor<Traversable>(std::forward<Traversable>(t)); } When you call adapt(lvalue) you get an adaptor<R&> and the member adaptor<R&>::range is a reference to the lvalue. This makes the lvalue case cheap as there's no copying. (This is what your adaptors do today.) When you call adapt(rvalue) you get an adaptor<R>and the member adaptor<R>::range is a copy of the rvalue, initialized by a move. The move construction is not as cheap as binding a reference, but it's safe and avoids a dangling reference. This would be very hard to do without rvalue references, and doesn't play nicely with make_iterator_range, because you don't want to make an iterator_range that refers to a temporary range, or you're back to the dangling reference problem. I've used this solution in isolated cases, but haven't got a generic solution to the problem.