Regression test relying on undefined compiler behavior (c++11)
libs/utility/numeric_traits_test.cpp generates values complement_traits<Number>::min using a clever recursive template, but that template relies on left-shifting a negative value, and according to the C++11 standard that’s a no-no (“the behavior is undefined”) which means it’s not a constant expression, which means it can’t be calculated at compile time, which means the BOOST_STATIC_ASSERT in line 332 won’t compile, saying “static_assert expression is not an integral constant expression” (I’m using clang++). See discussion here: http://stackoverflow.com/questions/23250651/strange-behavior-with-c-recursiv... In addition, it looks like the existing code also relies on the compiler filling the vacated bits with one’s, which I don’t think is what actually happens. The solution I’ve come up with, is to do the shifting using unsigned values, thus avoiding the compiler no-no, and then putting the result back in the specified type. In other words, replacing line 73 BOOST_STATIC_CONSTANT(Number, min = Number(Number(prev::min) << CHAR_BIT)); With BOOST_STATIC_CONSTANT(Number, min = boost::detail::is_signed<Number>::value ? (Number)(((unsigned long)(prev::min) << 8) | 0x00FF) : Number(Number(prev::min) << CHAR_BIT) ); I’m using constants ‘8’ and ‘0x00FF’ rather than CHAR_BIT since the code would get really ugly if CHAR_BIT != 8. Thoughts? Thanks, Chris
Actually, I came up with a better idea:
#ifndef BOOST_NO_LIMITS
template <class Number> struct complement_traits
{
BOOST_STATIC_CONSTANT(Number, min = std::numeric_limits<Number>::min());
BOOST_STATIC_CONSTANT(Number, max = std::numeric_limits<Number>::max());
};
#else
[SNIP] All the other template stuff for calculating complement_traits for various types
#endif
From: Chris Cooper
After the builds finally chugged through the build server, I see that std::numeric_limits<Number>::min() and std::numeric_limits<Number>::max() are not constant expressions with clang++ unless c++11 is enabled, so my second idea won’t quite work.
From: Chris Cooper
On Apr 23, 2014, at 4:48 PM, Chris Cooper
After the builds finally chugged through the build server, I see that std::numeric_limits<Number>::min() and std::numeric_limits<Number>::max() are not constant expressions with clang++ unless c++11 is enabled, so my second idea won’t quite work.
Try boost::integer_traits<Number>::const_min / const_max. Defined in boost/integer_traits.hpp.
[Trying to summarize the history of this discussion] libs/utility/numeric_traits_test.cpp generates values complement_traits<Number>::min using a clever recursive template, but that template relies on left-shifting a negative value, and according to the C++11 standard that’s a no-no ("the behavior is undefined") which means it’s not a constant expression, which means it can’t be calculated at compile time, which means the BOOST_STATIC_ASSERT in line 332 won’t compile, saying "static_assert expression is not an integral constant expression" (I’m using clang++). My final proposed solution: #if !defined(BOOST_NO_LIMITS) && !defined(BOOST_NO_CXX11_CONSTEXPR) template <class Number> struct complement_traits { BOOST_STATIC_CONSTANT(Number, min = std::numeric_limits<Number>::min()); BOOST_STATIC_CONSTANT(Number, max = std::numeric_limits<Number>::max()); }; #else [SNIP] All of the other template definitions for complement_traits, complement_traits_aux, etc. #endif Kim Barrett improved on this with: Try boost::integer_traits<Number>::const_min / const_max. Defined in boost/integer_traits.hpp. [end of summary] I have encapsulated this discussion into a boost bug: https://svn.boost.org/trac/boost/ticket/9944 Ben Pope, I had the same thought you did (about converting to an unsigned value before doing the left shift) but as I thought about it more, I dislike the whole algorithm that the author of that file (probably David Abrahams) was using. The assumptions implicit in that algorithm (e.g. (SCHAR_MIN << 8) == SHRT_MIN), while true for every platform I’ve worked on, I suspect are not actually part of the C++ standard so I think it’s best to not go down that path. I think it’s strange the author didn’t use std::numeric_limits or boost::integer_traits, it’s one of those times when I can’t tell whether the author is doing something incredibly clever (and deliberately not using std::numeric_limits and boost::integer_traits) because he is trying to test something specific, or whether the author got so caught up in creating a cool recursive template that he lost track of the actual testing …
On Thursday, April 24, 2014 01:59 AM, Chris Cooper wrote:
libs/utility/numeric_traits_test.cpp generates values complement_traits<Number>::min using a clever recursive template, but that template relies on left-shifting a negative value, and according to the C++11 standard that’s a no-no (“the behavior is undefined”) which means it’s not a constant expression, which means it can’t be calculated at compile time, which means the BOOST_STATIC_ASSERT in line 332 won’t compile, saying “static_assert expression is not an integral constant expression” (I’m using clang++).
See discussion here: http://stackoverflow.com/questions/23250651/strange-behavior-with-c-recursiv...
In addition, it looks like the existing code also relies on the compiler filling the vacated bits with one’s, which I don’t think is what actually happens.
The solution I’ve come up with, is to do the shifting using unsigned values, thus avoiding the compiler no-no, and then putting the result back in the specified type. In other words, replacing line 73
BOOST_STATIC_CONSTANT(Number, min = Number(Number(prev::min) << CHAR_BIT));
With
BOOST_STATIC_CONSTANT(Number, min = boost::detail::is_signed<Number>::value ? (Number)(((unsigned long)(prev::min) << 8) | 0x00FF) : Number(Number(prev::min) << CHAR_BIT) );
I’m using constants ‘8’ and ‘0x00FF’ rather than CHAR_BIT since the code would get really ugly if CHAR_BIT != 8.
Thoughts?
https://github.com/boostorg/utility/pull/4 http://thread.gmane.org/gmane.comp.lib.boost.devel/250143 Ben
participants (3)
-
Ben Pope -
Chris Cooper -
Kim Barrett