[Boost-bugs] [Boost C++ Libraries] #4081: gcc warns about "comparision always true" for basic_binary_oarchive

Subject: [Boost-bugs] [Boost C++ Libraries] #4081: gcc warns about "comparision always true" for basic_binary_oarchive
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2010-04-09 23:52:46

#4081: gcc warns about "comparision always true" for basic_binary_oarchive
------------------------------+---------------------------------------------
 Reporter:  kab@…             |       Owner:  ramey        
     Type:  Bugs              |      Status:  new          
Milestone:  To Be Determined  |   Component:  serialization
  Version:  Boost 1.42.0      |    Severity:  Problem      
 Keywords:                    |  
------------------------------+---------------------------------------------
 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/4081>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.