[serialization][1_42_0] basic_binary_oarchive - warning: comparison is always true due to limited range of data type
basic_binary_oarchive.hpp has this member: void save_override(const class_id_type & t, int){ // upto 32K classes assert(t.t <= boost::integer_traits<boost::int_least16_t>::const_max); const boost::int_least16_t x = static_cast<const boost::int_least16_t>(t.t); * this->This() << x; } It doesn't seem that there is a need for this runtime assert at all given the definition: BOOST_ARCHIVE_STRONG_TYPEDEF(int_least16_t, class_id_type) Can't the assert be removed to quiet the gcc warning? Jeff
Hmmm - what about a platform where int_least16_t is typedefed as a 32 bit integer? That is where int_least16_t can hold a value greater than 32K? Robert Ramey Jeff Flinn wrote:
basic_binary_oarchive.hpp has this member:
void save_override(const class_id_type & t, int){ // upto 32K classes assert(t.t <= boost::integer_traits<boost::int_least16_t>::const_max); const boost::int_least16_t x = static_cast<const boost::int_least16_t>(t.t); * this->This() << x; }
It doesn't seem that there is a need for this runtime assert at all given the definition:
BOOST_ARCHIVE_STRONG_TYPEDEF(int_least16_t, class_id_type)
Can't the assert be removed to quiet the gcc warning?
Jeff
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Apr 6, 2010, at 1:29 PM, Robert Ramey wrote:
Jeff Flinn wrote:
basic_binary_oarchive.hpp has this member:
void save_override(const class_id_type & t, int){ // upto 32K classes assert(t.t <= boost::integer_traits<boost::int_least16_t>::const_max); const boost::int_least16_t x = static_cast<const boost::int_least16_t>(t.t); * this->This() << x; }
It doesn't seem that there is a need for this runtime assert at all given the definition:
BOOST_ARCHIVE_STRONG_TYPEDEF(int_least16_t, class_id_type)
Can't the assert be removed to quiet the gcc warning?
Hmmm - what about a platform where int_least16_t is typedefed as a 32 bit integer? That is where int_least16_t can hold a value greater than 32K?
The test is still tautological, since it is asserting that a value of type T is not greater than the maximum value for the type T (where T in this case is int_least16_t). On such a platform the value of boost::integer_traits<boost::int_least16_t>::const_max would be the maximum value for that 32 bit integer type. There are several similarly tautological assertions nearby. One possibility is that the test bound that is actually wanted here is boost::integer_traits<boost::int16_t>::const_max so that 32K actually is the limit. Or perhaps instead it should be boost::integer_traits<short>::const_max, which is at least 32K, sticks to a primitive type, and is stylistically consistent with the assertion for the save_override for version_type, which uses unsigned char as the limiting type. Though the latter will likely still lead to the gcc warning at issue. Note that I ran into this same warning recently as part of upgrading to boost 1.42. Some research led me to write the following comment in my local patch: // kab, 4/2/2010: workaround for gcc warnings: // comparison is always true due to limited range of data type // gcc 4.3 and later provide warning controls for this: controled by // -W[no-]type-limits, defaulting to disabled and enabled by -Wextra. // gcc versions prior to that provide NO CONTROL over this warning, which // first appeared circa gcc 3.3.2. see gcc bug 12963. Frankly I'm hard-pressed to ever think of a situation where I'd want to enable this warning, but unfortunately there's no way to disable it for a significant range of gcc versions. For expedience I changed these asserts to use boost.numeric.conversion facilities to perform the range check, as it carefully optimizes out tests guaranteed to succeed, and so avoids triggering the warning in question. I'm guessing that a real patch that Robert would accept would avoid adding the dependency on boost.numeric.conversion, but that was good enough for my immediate purposes. Specifically, my patch introduces the following helper function: template<class T, class S> bool save_override_check(S s) { return boost::numeric::cInRange == boost::numeric::converter<T, S>::out_of_range(s); } which is used as assert(save_override_check<boost::int_least16_t>(t.t));
Note that I ran into this same warning recently as part of upgrading to boost 1.42. Some research led me to write the following comment in my local patch:
// kab, 4/2/2010: workaround for gcc warnings: // comparison is always true due to limited range of data type // gcc 4.3 and later provide warning controls for this: controled by // -W[no-]type-limits, defaulting to disabled and enabled by -Wextra. // gcc versions prior to that provide NO CONTROL over this warning, which // first appeared circa gcc 3.3.2. see gcc bug 12963.
Frankly I'm hard-pressed to ever think of a situation where I'd want to enable this warning, but unfortunately there's no way to disable it for a significant range of gcc versions.
For expedience I changed these asserts to use boost.numeric.conversion facilities to perform the range check, as it carefully optimizes out tests guaranteed to succeed, and so avoids triggering the warning in question. I'm guessing that a real patch that Robert would accept would avoid adding the dependency on boost.numeric.conversion, but that was good enough for my immediate purposes. Specifically, my patch introduces the following helper function:
template<class T, class S> bool save_override_check(S s) { return boost::numeric::cInRange == boost::numeric::converter<T, S>::out_of_range(s); }
which is used as
assert(save_override_check<boost::int_least16_t>(t.t));
Well, this discussion makes me want to re-consider - at my leasure - what kind of checking - if any - should be done. It touches upon other issues as well, such as my imposed requirement that all archives should act identical. Feel free to open a track item on this so that it will hang around forever or until it's specifically considerd - whichever comes first. Robert Ramey
On Apr 9, 2010, at 1:26 PM, Robert Ramey wrote:
Well, this discussion makes me want to re-consider - at my leasure - what kind of checking - if any - should be done. It touches upon other issues as well, such as my imposed requirement that all archives should act identical. Feel free to open a track item on this so that it will hang around forever or until it's specifically considerd - whichever comes first.
participants (3)
-
Jeff Flinn -
Kim Barrett -
Robert Ramey