Opened 13 years ago

Closed 11 years ago

Last modified 11 years ago

#3659 closed Bugs (invalid)

warning when using boost::counting_iterator<int> and std::vector<int> on MSVC

Reported by: anonymous Owned by: Dave Abrahams
Milestone: Boost 1.42.0 Component: iterator
Version: Boost 1.41.0 Severity: Cosmetic
Keywords: Cc:

Description

This code causes the warnings:

#include <boost/iterator/counting_iterator.hpp> #include <vector>

int main()

{ boost::counting_iterator<int>

begin, end;

std::vector<int> v(begin, end);

return 0; }

warning C4244: '+=' : conversion from 'int64' to 'unsigned int', possible loss of data 1> e:\program files\microsoft visual studio 8\vc\include\xutility(1702) : see reference to function template instantiation 'void std::_Distance2<_InIt,_Diff>(_RanIt,_RanIt,_Diff &,std::random_access_iterator_tag)' being compiled 1> with 1> [ 1> _InIt=boost::counting_iterator<int>, 1> _Diff=unsigned int, 1> _RanIt=boost::counting_iterator<int> 1> ]

Change History (10)

comment:1 by Steven Watanabe, 12 years ago

Resolution: wontfix
Status: newclosed

I don't think this is fixable. The difference type of counting_iterator is deliberately set to intmax_t, so that any difference can be represented. You can explicitly set the difference type to something else, if you really want to.

comment:2 by Sergey Mitsyn <svm at jinr.ru>, 11 years ago

The warning still doesn't go when the difference type is explicitly specified instead of the default:

#include <boost/iterator/counting_iterator.hpp>
int main()
{
	boost::counting_iterator<int, boost::use_default, int> a(0), b(10);

 	a+=(b-a);

	return 0;
}

the compiler complains with the following:

boost_1_43_0\boost\iterator\counting_iterator.hpp(139) : warning C4244: 'return' : conversion from '__int64' to 'int', possible loss of data

IMHO looks like the C-style cast to numeric_distance forces the difference to be intmax_t, and implicit conversion to return type Difference invokes the warning.

Maybe it's OK to assume that if a user specifies the difference type explicitly, he/she knows what he/she is doing and use static_cast to Difference ?

comment:3 by anonymous, 11 years ago

Resolution: wontfix
Status: closedreopened

in reply to:  2 comment:4 by Dave Abrahams, 11 years ago

Replying to Sergey Mitsyn <svm at jinr.ru>:

IMHO looks like the C-style cast to numeric_distance forces the difference to be intmax_t, and implicit conversion to return type Difference invokes the warning.

There's no C-style cast. That's calling numeric_distance from boost/detail/numeric_traits.hpp.

in reply to:  2 ; comment:5 by Dave Abrahams, 11 years ago

Resolution: invalid
Status: reopenedclosed

Replying to Sergey Mitsyn <svm at jinr.ru>:

The warning still doesn't go when the difference type is explicitly specified instead of the default:

Maybe it's OK to assume that if a user specifies the difference type explicitly, he/she knows what he/she is doing and use static_cast to Difference ?

Maybe the warning is a good thing. I object most strongly to introducing casts to suppress warnings, especially in libraries: casts hide bugs. If you can find a way to make the warning go away without casting, I'll accept a patch. Otherwise, I think the compiler may be telling you that you're doing something you shouldn't do, and this is not a bug in the library.

in reply to:  5 ; comment:6 by Sergey Mitsyn <svm at jinr.ru>, 11 years ago

Replying to dave:

There's no C-style cast. That's calling numeric_distance from boost/detail/numeric_traits.hpp.

Yes, sorry, I've missed that.

Otherwise, I think the compiler may be telling you that you're doing something you shouldn't do, and this is not a bug in the library.

IMHO taking a difference between two iterators is not a thing everybody shouldn't do, which is the case if Difference type is specified as int. And adding a difference between two iterators to an iterator of the same type is not a thing everybody shouldn't do either, which is the case with default Difference type.


Ok, i've investigated the problem a little more. The actual problem i'm faced is warning in the following code:

typedef boost::counting_iterator<int, boost::use_default> iterator_type;
//typedef boost::counting_iterator<int, boost::bidirectional_traversal_tag> iterator_type;
iterator_type a(0), b(10);
std::vector<int> v(10, 0);
std::copy( a,b,v.begin() );

The actual warning may be reproduced by this code (inside std::copy implementation):

std::vector<int>::iterator it = v.begin();
it += b-a;

The std::allocator<T>::difference_type that is used by std::vector is defined as ptrdiff_t, which is int for 32-bit target. Thus, std::vector::iterator::operator+= has 32-bit integer as an argument.

On the other hand, boost::integer_traits defines difference_type to be 64-bit intmax_t if Integer and ptrdiff_t are the same size, which is the case. Thus, the return type of operator- for boost::counting_iterator is 64-bit integer. Passing it to operator+= causes the warning.

What I'm thinking about now is the fact that Dinkumware used 32-bit integer for 32-bit machine architecture instead of 64-bit, thus ignoring the requirement to represent any difference, as Steven stated above. Maybe Boost could also switch to using 32-bit ptrdiff_t with the same reason as Dinkumware does, at least for MSVC?


Also the interesting fact is that for boost::counting_iterator<short> no warning is shown, because it's difference_type is ptrdiff_t. But such workaround is unacceptable when there's a need in values bigger than 32767 :(.

in reply to:  6 ; comment:7 by Dave Abrahams, 11 years ago

Replying to Sergey Mitsyn <svm at jinr.ru>:

What I'm thinking about now is the fact that Dinkumware used 32-bit integer for 32-bit machine architecture instead of 64-bit, thus ignoring the requirement to represent any difference, as Steven stated above. Maybe Boost could also switch to using 32-bit ptrdiff_t with the same reason as Dinkumware does, at least for MSVC?

ptrdiff_t is supplied by the standard library (i.e. Dinkumware) and not defined by Boost. We use whatever definition we get from the library.

in reply to:  7 comment:8 by Sergey Mitsyn <svm at jinr.ru>, 11 years ago

Replying to dave:

Replying to Sergey Mitsyn <svm at jinr.ru>:

What I'm thinking about now is the fact that Dinkumware used 32-bit integer for 32-bit machine architecture instead of 64-bit, thus ignoring the requirement to represent any difference, as Steven stated above. Maybe Boost could also switch to using 32-bit ptrdiff_t with the same reason as Dinkumware does, at least for MSVC?

ptrdiff_t is supplied by the standard library (i.e. Dinkumware) and not defined by Boost. We use whatever definition we get from the library.

Sorry, I meant switching from intmax_t to ptrdiff_t, not redefining ptrdiff_t. What I meant to say is that boost::counting_iterator should move away from 'no-overflow' strategy, at least for MSVC + x86 and x64, because it would be more consistent with the Dinkumware STL implementation.

Anyway, looks like it would be much easier to reimplement counting iterator with overflow-able strategy, or implement 'int' wrapper which plays nice with {{{boost::counting_iterator}} rather than fixing this "bug" in Boost.

comment:9 by Sergey Mitsyn <svm at jinr.ru>, 11 years ago

BTW, not every difference is representable even with current strategy. In this code, difference overflows without issuing any warning:

typedef boost::counting_iterator<boost::intmax_t> iterator_type;
iterator_type a(-0x7ffFfffFfffFfffF), b(0x7ffFfffFfffFfffF);

std::cout << *a << '\t' << *b << std::endl;
std::cout << (b-a) << std::endl;

, the output is:

-9223372036854775807    9223372036854775807
-2

Thus boost::counting_iterator cannot provide guarantee that (b-a) cannot overflow for any Incrementable. Plus, I believe that everybody would expect (b-a) to be equal to (*b-*a).

Please, switch to strategy with overflows. :)

comment:10 by Sergey Mitsyn <svm at jinr.ru>, 11 years ago

Hello again,

I've found http://lists.boost.org/Archives/boost/2008/03/134556.php. Okay, now the reason to set difference_type to be intmax_t is perfectly clear to me. Also, I see that boost::counting_iterator<intmax_t> may trigger undefined behavior (as in the example I listed above).

But still i would like to have an option for boost::counting_iterator (or another counting iterator that is not called exactly boost::counting_iterator) so it does not spam my console with compiler warnings.

wbr,

Note: See TracTickets for help on using tickets.