unhex level 4 compile warning in visual studio

[Gary Sanders <lex21@…>]

The following code generates a level 4 compiler warning in Visual Studio 2010 and 2012:

#include <string>
#include <boost/algorithm/hex.hpp>

int main()
{
    std::string base = "now is the time";
    std::string hstr = boost::algorithm::hex(base);
    std::string cstr = boost::algorithm::unhex(hstr);
}

The warnings are:

1>d:\development\libs\boost_1_52_0\boost/algorithm/hex.hpp(137): warning C4244: '=' : conversion from 'unsigned int' to 'T', possible loss of data

1>          d:\development\libs\boost_1_52_0\boost/algorithm/hex.hpp(204) : see reference to function template instantiation 'std::back_insert_iterator<_Container> boost::algorithm::detail::decode_one<InputIterator,OutputIterator,bool(__cdecl *)(Iterator,Iterator)>(InputIterator &,InputIterator,OutputIterator,EndPred)' being compiled
1>          with
1>          [
1>              _Container=std::string,
1>              InputIterator=std::_String_const_iterator<char,std::char_traits<char>,std::allocator<char>>,
1>              OutputIterator=std::back_insert_iterator<std::string>,
1>              Iterator=std::_String_const_iterator<char,std::char_traits<char>,std::allocator<char>>,
1>              EndPred=bool (__cdecl *)(std::_String_const_iterator<char,std::char_traits<char>,std::allocator<char>>,std::_String_const_iterator<char,std::char_traits<char>,std::allocator<char>>)
1>          ]
1>          d:\development\libs\boost_1_52_0\boost/algorithm/hex.hpp(237) : see reference to function template instantiation 'OutputIterator boost::algorithm::unhex<std::_String_const_iterator<_Elem,_Traits,_Alloc>,OutputIterator>(InputIterator,InputIterator,OutputIterator)' being compiled
1>          with
1>          [
1>              OutputIterator=std::back_insert_iterator<std::string>,
1>              _Elem=char,
1>              _Traits=std::char_traits<char>,
1>              _Alloc=std::allocator<char>,
1>              InputIterator=std::_String_const_iterator<char,std::char_traits<char>,std::allocator<char>>
1>          ]
1>          d:\development\libs\boost_1_52_0\boost/algorithm/hex.hpp(263) : see reference to function template instantiation 'OutputIterator boost::algorithm::unhex<String,std::back_insert_iterator<_Container>>(const Range &,OutputIterator)' being compiled
1>          with
1>          [
1>              OutputIterator=std::back_insert_iterator<std::string>,
1>              String=std::string,
1>              _Container=std::string,
1>              Range=std::string
1>          ]
1>          unhex_bug.cpp(10) : see reference to function template instantiation 'String boost::algorithm::unhex<std::string>(const String &)' being compiled
1>          with
1>          [
1>              String=std::string
1>          ]

[Marshall Clow]

This is a crap warning.

There is no loss of data here.

The routine hex_char_to_int only returns values in the range 0..15.

With T == char, the loop at line 134 will be executed twice.

res is initialized to zero.

The first time through the loop, we multiply res (0) * 16 + add the result of hex_char_to_int, so the maximum value that res can be is 15.

The second time through the loop, we multiply res * 16 and add the result of hex_char_to_int, so the maximum value that res can be is (15*16) + 15, or 255.

No overflow; no loss of data.

Please file a bug report with your compiler vendor.

[Gary Sanders <lex21@…>]

I agree that there is no loss of data with regard to the compiler warning.

Nevertheless, the attached patch silences the compiler warning referenced above, as well as another compiler warning that indicates that there is unreachable code in hex_char_to_int (the return 0 after the exception is thrown).

[Marshall Clow]

The return that you've added to the end of hex_char_to_int is still unreachable. Other compilers will (potentially) warn on that.

You're proposing a substantial pessimization of the code base to eliminate a warning where the compiler is wrong.

[Gary Sanders <lex21@…>]

The structure of the proposed patch is as follows:

template <typename T> T hex_char_to_int ( T c ) {
  T r;
  if      (...) r = ...;
  else if (...) r = ...;
  else if (...) r = ...;
  else    BOOST_THROW_EXCEPTION (...);
  return r;
}

There is one and only one return point. Therefore, the return cannot be un-reachable.

The proposed patch, verified on GCC 4.6.3, Visual Studio 2010 and Visual Studio 2012, does not emit warnings. Furthermore, it removes the need for hex_char_to_int reside an anonymous namespace, which could cause link errors if used within multiple translation units.

The reality is that even if Microsoft agreed to change their compiler, it will not be done for Visual Studio 2010 or 2012. In the spirit of the last comment in hex_char_to_in, ('keep dump compilers happy') it would be very much appreciated if you would endeavour to keep Visual Studio happy, too.

[pmost@…]

I'm still experiencing this bug in Boost 1.58.0 with Visual Studio 2013 compiler warning level 4. The problematic code is in hex.hpp(74)

    template <typename T>
    unsigned char hex_char_to_int ( T val ) {
        char c = static_cast<char> ( val );
        unsigned retval = 0;
        if      ( c >= '0' && c <= '9' ) retval = c - '0';
        else if ( c >= 'A' && c <= 'F' ) retval = c - 'A' + 10;
        else if ( c >= 'a' && c <= 'f' ) retval = c - 'a' + 10;
        else BOOST_THROW_EXCEPTION (non_hex_input() << bad_char (c));
        return retval;
        }

If I define retval to be unsigned char then this warning goes away. Wouldn't this be the simplest solution?