#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_distance
forces the difference to be intmax_t, and implicit conversion to return typeDifference
invokes 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_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 , 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.