id,summary,reporter,owner,description,type,status,milestone,component,version,severity,resolution,keywords,cc
4081,"gcc warns about ""comparision always true"" for basic_binary_oarchive",kab@…,Robert Ramey,"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.

",Bugs,closed,To Be Determined,serialization,Boost 1.42.0,Problem,fixed,,