[Sorry, forgot to reply-all.] ---------- Forwarded message ---------- From: Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung@gmail.com> Date: Fri, Nov 30, 2012 at 5:53 PM Subject: Re: [boost][iterator] UB when implicitly using default constructed counting_iterator<unsigned> To: Peter Sommerlad <peter.sommerlad@hsr.ch> On Fri, Nov 30, 2012 at 1:14 AM, Peter Sommerlad <peter.sommerlad@hsr.ch>wrote:
Hi Jeffrey, hi all,
I believe the usability of counting iterator could be improved, by value-initializing its Iterator type in the default constructor, because integral types won't be initialized by its default constructor. This can lead to hard to recognize user errors, e.g., when wrapping such a counting iterator with a filter iterator.
counting_iterator is special in that sense, because to my understanding it is intended to adapt integral types to become iterators. And a default initialized scalar type, isn't.
"isn't"...what, exactly?
I suggest a one line change:
counting_iterator() { }
should become
counting_iterator() : super_t(Incrementable()) { }
Well, that's one thing you can do...
The following example program shows the interesting UB, because the 3rd argument of make_filter_iterator, default constructs a counting_iterator<unsigned>().
Any thoughts? Objections? Teachings...
Regards Peter.
=====>8 #include <iostream> #include <boost/iterator/filter_iterator.hpp> #include <boost/iterator/counting_iterator.hpp> #include <algorithm> #include <iterator> #include <functional>
struct is_prime{ bool operator()(unsigned int val) const { if (val < 2) return false; using std::placeholders::_1; auto ce=boost::make_counting_iterator(val); return ce==std::find_if_not(boost::make_counting_iterator(2u),ce,
bind(std::modulus<unsigned>{},val,_1)); } }; int main(){
std::copy_n(boost::make_filter_iterator(is_prime{},boost::make_counting_iterator(1u) // if the following line is omitted we have undefined behavior, because of uninitialized unsigned value // ,boost::make_counting_iterator(std::numeric_limits<unsigned>::max()) // must be different from above value ),40,std::ostream_iterator<unsigned>{std::cout,", "}); }
=====8<
The thing that bothers me about this is that a default-constructed iterator frequently is an end iterator; indeed, that's the assumption that filter_iterator appears to be making if no end iterator is supplied. So if the default-constructed iterator is not an end iterator, and you're wrapping it with a filter_iterator and not explicitly supplying the end, I'd say you're automatically asking for trouble, regardless of whether you're using a counting_iterator or not. On the other hand, I'm not sure what a sensible default value is for counting_iterator< [integral type] >; 0 is almost never going to be the end value; std::numeric_limits<>::max() might be a better choice, but really the end value is context-dependent. I'm not opposed to giving defined behavior to a default constructed counting_iterator< [integral type] >, I'm just not sure what that defined behavior should be. Maybe I'm over-analyzing it... - Jeff