[iterator] UB when implicitly using default constructed counting_iterator<unsigned>
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. I suggest a one line change: counting_iterator() { } should become counting_iterator() : super_t(Incrementable()) { } 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< -- Prof. Peter Sommerlad Institut für Software: Bessere Software - Einfach, Schneller! HSR Hochschule für Technik Rapperswil Oberseestr 10, Postfach 1475, CH-8640 Rapperswil http://ifs.hsr.ch http://cute-test.com http://linticator.com http://includator.com tel:+41 55 222 49 84 == mobile:+41 79 432 23 32 fax:+41 55 222 46 29 == mailto:peter.sommerlad@hsr.ch
On 11/30/2012 4:14 AM, Peter Sommerlad wrote:
Hi Jeffrey, hi all, ... 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,", "});
IMHO, the counting_iterator concern is misplaced. The true culprit is make_filter_iterator's defaulted end iterator argument. Of all the iterator types in the standard library, only istream[buf]_iterator(as pointed out by Steven Watanabe) is valid in this context. My preferences is to leave counting_iterator as is, and remove the default from make_filter_iterator. Problematic code would be caught at compile time. Valid uses would require explicit provision of istream end iterators. But of course it may be that this horse has already left the barn. In fact the advent of Range adaptors and algorithms has obviated this problem. Jeff
On 11/30/2012 4:14 AM, Peter Sommerlad wrote:
Hi Jeffrey, hi all, ... 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,", "});
IMHO, the counting_iterator concern is misplaced. The true culprit is make_filter_iterator's defaulted end iterator argument. Of all the iterator types in the standard library, only istream[buf]_iterator(as pointed out by Steven Watanabe) is valid in this context.
My preferences is to leave counting_iterator as is, and remove the default from make_filter_iterator.
+1 Regards, Nate
participants (3)
-
Jeff Flinn -
Nathan Ridge -
Peter Sommerlad