Opened 13 years ago

Closed 12 years ago

#4081 closed Bugs (fixed)

gcc warns about "comparision always true" for basic_binary_oarchive

Reported by: kab@… Owned by: Robert Ramey
Milestone: To Be Determined Component: serialization
Version: Boost 1.42.0 Severity: Problem
Keywords: Cc:

Description

When upgrading to boost 1.42 I (and some other users too) ran into a problem with spurious warnings from (some versions of) gcc.

Several of the save_override members of basic_binary_oarchive contain asserts to verify that the value about to be written is within a certain bound. Some of the test are in actual fact tautological, since they are of the form "x <= numeric_limits<T>::const_max" with x being of type T.

Ordinarily that wouldn't be a big deal, since the compiler can simply optimize away the test, and it provides at least some documentation of intent, and may provide some benefit against future changes elsewhere leading to unexpected problems.

Unfortunately, for a substantial range of gcc versions, gcc issues a warning for such a tautological test, and worse yet, provides no mechanism for controlling the generation of these warnings. The specific warning is

comparison is always true due to limited range of data type

This is the subject of gcc bug 12963. The problematic version range appears to be from gcc 3.3.2 up to but not including gcc 4.3. From gcc 4.3 onward this is controlled by -W[no-]type-limits, which defaults to disabled and is enabled by -Wextra.

Frankly I'm hard-pressed to think of a situation where I'd want to enable this warning, but unfortunately there's simply no way to disable it prior to gcc 4.3.

I've locall worked around this gcc bug by patching the asserts in question to use boost.numeric.conversion to perform the range check, as it carefully optimizes out tests guaranteed to succeed, and so avoids triggering the warning in question. Specifically, my patch introduces the following helper function for use as the assertion test:

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));

This was a sufficient solution for my local patch. However, it might not be desirable to add boost.numeric.conversion to the serialization library's dependencies just for this. A different approach would involve adding a value_type typedef to the classes defined by BOOST_STRONG_TYPEDEF, and change the tests to something like (with appropriate addition of "typename" and namespace qualifiers):

template<class T, class S> bool save_override_check(S s) {

return (numeric_limits<S::value_type>::const_max

<= numeric_limits<T>::const_max)

(s.t <= boost::numeric_limits<T>::const_max);

}

Hopefully in this situation gcc would not warn about the now unreachable tautological comparision. I presume it would not complain about the comparison of two constants.

The addition of a value_type typedef to the strong-typedef mechanism has some appeal independently of this, just for generally allowing some introspection on such types.

My local patch didn't take the value_type approach because that would involve patching more files, and because I didn't have time to actually try it and perhaps discover that gcc was more stupid than I expected about this warning and still issued it in that situation.

In followup discussion of this issue on the boost list, Robert Ramey had this to say:

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.

Change History (1)

comment:1 by Robert Ramey, 12 years ago

Resolution: fixed
Status: newclosed

I took these asserts out.

Robert Ramey

Note: See TracTickets for help on using tickets.