[lmi] [PATCH] Fix value_cast defect shown by the unit test

[Vadim Zeitlin]

 Hello,

 A long time ago I reported a problem in value_cast_test.cpp with gcc 4.1,
see http://lists.nongnu.org/archive/html/lmi/2008-06/msg00010.html
Unfortunately I was too quick to believe that this problem was due to a
compiler bug (which is, admittedly, understandable as gcc 4.1 was quite
buggy) and didn't investigate it fully. However I returned to it now after
getting warnings (i.e. errors, due to lmi use of -Werror) when compiling
this test with gcc 6.3:

---------------------------------- >8 --------------------------------------
In file included from value_cast.hpp:32:0,
                 from value_cast_test.cpp:24:
src/3rdparty/boost_1_33_1/boost/type_traits/is_convertible.hpp: In 
instantiation of 'const bool boost::detail::is_convertible_basic_impl<char 
(&)[2], bool>::value':
src/3rdparty/boost_1_33_1/boost/type_traits/is_convertible.hpp:228:5:   
required from 'const bool boost::detail::is_convertible_impl<char [2], 
bool>::value'
src/3rdparty/boost_1_33_1/boost/type_traits/is_convertible.hpp:348:1:   
required from 'struct boost::is_convertible<char [2], bool>'
value_cast.hpp:186:9:   required from 'struct value_cast_choice<bool, char [2]>'
value_cast.hpp:254:12:   required from 'To value_cast(const From&) [with To = 
bool; From = char [2]]'
value_cast_test.cpp:743:5:   required from here
src/3rdparty/boost_1_33_1/boost/type_traits/is_convertible.hpp:127:68: error: 
the compiler can assume that the address of 
'boost::detail::is_convertible_basic_impl<char (&)[2], bool>::_m_from' will 
always evaluate to 'true'  -Werror=address]
     static bool const value = sizeof( detail::checker<To>::_m_check(_m_from, 
0) )
                                     
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
In file included from value_cast_test.cpp:24:0:
value_cast.hpp: In instantiation of 'To value_cast_chooser<To, From, 
1>::operator()(const From&) [with To = bool; From = char [2]]':
value_cast.hpp:254:41:   required from 'To value_cast(const From&) [with To = 
bool; From = char [2]]'
value_cast_test.cpp:743:5:   required from here
value_cast.hpp:233:74: error: the compiler can assume that the address of 
'from' will always evaluate to 'true' [-Werror=address]
     To operator()(From const& from) {throw_if_null_pointer(from); return from;}
                                                                          ^~~~
value_cast.hpp: In member function 'To value_cast_chooser<To, From, 
1>::operator()(const From&) [with To = bool; From = char [2]]':
value_cast.hpp:233:74: error: nonnull argument 'from' compared to NULL 
[-Werror=nonnull-compare]
     To operator()(From const& from) {throw_if_null_pointer(from); return from;}
                                                                          ^~~~
---------------------------------- >8 --------------------------------------

 The last two warnings show that the compiler selects to use e_direct
specialization of value_cast_chooser and this means that
value_cast_choice::convertible is true, which is indeed the case because
the type of the literal "1" in the unit test, i.e. "char*" is convertible
to bool via the unfortunate inherited from C pointer-to-zero-or-not
conversion.

 But what this means is that using value_cast<bool>("string") basically
always return true, which definitely doesn't seem to be the intention of
this code. I.e. while the current test passes, if you add

    BOOST_TEST_EQUAL(false, value_cast<bool>("0"));

to the test, it would fail (both with gcc 6 and clang, even though the
latter doesn't give any warnings when compiling), contrary to the existing
(passing) test

    BOOST_TEST_EQUAL(false, value_cast<bool>(std::string("0")));

So the problem here is worse than just warnings in the test, value_cast<>
doesn't work correctly in this case. It doesn't seem like this combination
of types is used anywhere else in the unit test however, so it's not really
a defect, but still worth fixing IMHO.

 The simplest fix I can propose is just testing for !is_array<>, however
this still leaves us with the first warning, from Boost type traits
library. I don't see any simple way to avoid it and I definitely don't want
to disable the useful -Waddress warning, so instead I propose to just
switch to the standard C++11 <type_traits> which doesn't suffer from this
problem.

 The patch at 

        https://github.com/vadz/lmi/pull/50

contains both changes: the switch to standard functions in the first commit
and the fix for the problem in the second one. I've tested this with gcc
6.3, clang (pre-4.0) and MinGW 4.9.1 used for the official builds, the test
now passes with all of them, including the newly re-enabled test.

 Please let me know if you have any questions about this,
VZ