Re: [boost] [iterator] UB when implicitly using default constructed counting_iterator<unsigned>

Subject: Re: [boost] [iterator] UB when implicitly using default constructed counting_iterator<unsigned>
From: Claas H. Köhler (claas.koehler_at_[hidden])
Date: 2012-12-03 09:07:34

On 01/12/12 18:39, Steven Watanabe wrote:
> AMDG
> 
> On 12/01/2012 07:20 AM, Peter Sommerlad wrote:
>> Hi Jeff
>>
>> On 01.12.2012, at 02:53, Jeffrey Lee Hellrung, Jr. wrote:
>>
>>>>  And a default
>>>> initialized scalar type, isn't.
>>>>
>>>
>>> "isn't"...what, exactly?
>>
>> it isn't initialized with a defined value.
>>
>> struct X{
>> 	X(){}
>> unsigned x;
>> };
>> void foo(){
>> X y; // -> y.x has an uninitialized value AFAIK if it is local
>> }
>> that is the situation we run into with the default constructed counting_iterator<unsigned>()
>>
> 
> IMO, this is a good thing.  Any use of a default
> constructed counting_iterator is probably wrong,
> no matter what (arbitrary) value is chosen.  As
> such, it's better to leave it uninitialized, so
> these errors can be caught by tools like valgrind.
> 
>>> 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...
>>
>> I still believe the most simple change I suggested is enough (at least it would cover my situation well enoughi).
> 
> Your use case is very error prone.  It only works
> because you started the iteration at 1, not 0.
> 
>> Those input iterators where a default constructed one is the end iterator is IMHO more of an exception, because it is a special case for the stream iterators.
> 
> Any other iterator gives undefined behavior when
> it's default constructed.  I don't see why counting_iterator
> should be different.
> 
>> Your idea with std::numberic_limits< Iterator >::max() requires a much more complex solution with template meta programming, i.e., enable_if<is_arithmetic<Iterator>> or similar. And it won't provide the default value of Iterator, which is zero in the case of the built-in arithmetic types, i.e., unsigned().
>>
>> What do you think?
>>
> 
> In Christ,
> Steven Watanabe

Accidentally I just encountered a similar problem in my code, which was caused by counting_iterator
not beeing initialised and it took me a while to debug it.

I support Peter's suggestion to initialise counting_iterator<T> with T() because it forwards the
default behaviour of T, which will become inaccessible in template code otherwise.

Consider e.g. a simple template of the form

template<class IT>
struct Range {
  IT first, last;

  Range(void)= default;

  size_t size(void) const {return last - first;}
};

The behaviour of size() is currently undefined for default constructed Range objects, which is a
major disadvantage in my opinion, since any reproducible default value will result in the expected
behaviour of size zero for a default constructed Range object. This is what all other iterator
implementations guarantee, too, if I am not mistaken (Please correct me if I am wrong here).

Furthermore I interpret counting_iterator as a wrapper around objects of type T, so forwarding the
default constructor of T is desirable from my point of view, mainly because there is no way to
reproduce its behaviour, once the information about the iterator type is lost (as is the case in the
example above)

Just my two cents.

Regards
Claas