#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 , 12 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
follow-ups: 4 5 comment:2 by , 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 , 11 years ago
| Resolution: | wontfix |
|---|---|
| Status: | closed → reopened |
comment:4 by , 11 years ago
Replying to Sergey Mitsyn <svm at jinr.ru>:
IMHO looks like the C-style cast to
numeric_distanceforces the difference to be intmax_t, and implicit conversion to return typeDifferenceinvokes the warning.
There's no C-style cast. That's calling numeric_distance from boost/detail/numeric_traits.hpp.
follow-up: 6 comment:5 by , 11 years ago
| Resolution: | → invalid |
|---|---|
| Status: | reopened → closed |
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.
follow-up: 7 comment:6 by , 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 :(.
follow-up: 8 comment:7 by , 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.
comment:8 by , 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_tis 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 , 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 , 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,

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.