This is needed anytime we need to clamp an arbitrary floating point value to an integer type.
Details
 Reviewers
mclow.lists scanon EricWF zoecarver  Commits
 rGe8316372b91e: [libc++] Add `__truncating_cast` for safely casting float types to integers
rCXX370891: [libc++] Add `__truncating_cast` for safely casting float types to integers
rL370891: [libc++] Add `__truncating_cast` for safely casting float types to integers
Diff Detail
 Repository
 rL LLVM
Event Timeline
Seems like a very useful function. __max_representable_int_for_float also seems useful. Should this work in C++03? If so there are a few changes that need to be made. It would also be great if this could be a constexpr (but, obviously, not necessary).
include/math.h  

1556 ↗  (On Diff #217513)  Seems odd this is the only thing in this file inside the standard namespace. Are we moving towards writing std::__helper instead of __libcpp_helper? It seems like the other helper functions in this file use the __libcpp prefix and aren't in the standard namespace. 
1558 ↗  (On Diff #217513)  Nit: maybe qualify all the uses of numeric_limits and similar? 
1572 ↗  (On Diff #217513)  What is the enum providing for you? Couldn't this just be static const int _Bits = ...? 
1573 ↗  (On Diff #217513)  What's the reasoning behind shifting something forward and back? Shouldn't this always negate the other operation? 
1579 ↗  (On Diff #217513)  This will not work before C++11. 
1582 ↗  (On Diff #217513)  Maybe change INFINITY to std::numeric_limits< _RealT >::infinity() 
test/libcxx/numerics/truncating_cast.pass.cpp  
10 ↗  (On Diff #217513)  Is this supposed to work in C++03? If so, update this test and __truncating_cast. Otherwise, add an #if and a // UNSUPPORTED: C++98, C++03 
25 ↗  (On Diff #217513)  Maybe test with more than just double. float, long double, others? 
28 ↗  (On Diff #217513)  C++03 will not like this :P 
I would tend to write this function in the following form:
// set up lower bound and upper bound if (r > upperBound) r = upperBound; if (!(r >= lowerBound)) r = lowerBound; // NaN is mapped to l.b. return static_cast<IntType>(r);
I prefer to avoid the explicit trunc call, since that's the defined behavior of the static_cast once the value is inrange, anyway.
include/math.h  

1573 ↗  (On Diff #217513)  This function doesn't quite do what it says on the tin; it considers the number of significand bits used for the floatingpoint type, but not the exponent range. This doesn't matter for double, because double's exponent range is much, much larger than any integer type, but it does matter for types like float16 (largest representable value is 65504)when it's added as a standard floatingpoint type at some future point, this will introduce subtle bugs. You should be able to work around this by converting value to _FloatT, taking the minimum of the result and numeric_limits::max, and converting back. This also assumes that _FloatT has radix == 2, which I do not believe is actually implied by is_floating_point == true. Please add a static assert for that so that future decimal types don't use this template. 
1582 ↗  (On Diff #217513)  Why isn't this just __trunc_r > _MaxVal? 
1584 ↗  (On Diff #217513)  This has a subtle assumption that _IntT is two'scomplement and _FloatT has radix=2, so that the implicit conversion that occurs in the comparison is exact. The radix should be a static assert; does libc++ care about nontwo'scomplement at all? Just from a clarity perspective, I would personally make the conversion explicit. 
1586 ↗  (On Diff #217513)  If I'm reading right, NaNs will fall through the above two comparisons and invoke UB on the static_cast below. I suspect that's not the desired behavior. What is the intended result for NaN? 
test/libcxx/numerics/truncating_cast.pass.cpp  

36 ↗  (On Diff #217513)  Probably should test nextafter(static_cast<double>(Lim::max()), INFINITY) here instead. 
include/math.h  

1556 ↗  (On Diff #217513)  We shouldn't put things in the global namespace generally. It's fine that this is the only thing in namespace std is this file. 
1558 ↗  (On Diff #217513)  No need to qualify types. The lookup will be unambiguous. 
1572 ↗  (On Diff #217513)  It subtly enforces that the argument is a compile time constant without introducing a global variable declaration. 
1579 ↗  (On Diff #217513)  Using statements? Yes it will, but only because we require Clang in C++03 now. And Clang allows using statements as an extension. 
1582 ↗  (On Diff #217513)  Consider long long and double. MaxVal  numeric_limits<long long>::max() == 1024, and we want values between MaxVal and ::max() to round down. So instead we essentially check for __r >= numeric_limits<long long>::max() + 1. This approach seems more accurate. 
1584 ↗  (On Diff #217513)  I'll static assert the radix, but I think it's safe to assume twos compliment. According to p0907, none of MSVC, GCC, or Clang support other representations [1]. How would you make this explicit? http://www.openstd.org/jtc1/sc22/wg21/docs/papers/2018/p0907r0.html 
1586 ↗  (On Diff #217513)  I didn't want to treat NaN as a valid input, so I want to allow UBSAN to catch it rather than to provide a valid output. 
test/libcxx/numerics/truncating_cast.pass.cpp  
10 ↗  (On Diff #217513)  The test compiles and passes with C++03. 
include/math.h  

1582 ↗  (On Diff #217513) 
Yes, but there are no values between MaxVal and ::max() in the floatingpoint format; if there were, they would be MaxVal instead. So you can ditch the nextafter and just use > static_cast<_FloatT>(MaxVal). 
include/math.h  

1586 ↗  (On Diff #217513)  Please document this clearly; otherwise someone will assume that this is a UBfree conversion and use it for that purpose. 
include/math.h  

1573 ↗  (On Diff #217513)  Very good point. I've added the static assertions and limited the function to float, double, and long double so the fp16 case won't bite us anytime soon. 
include/math.h  

1582 ↗  (On Diff #217513)  I thought the same thing, but that isn't necessarily true. Eric showed me this link https://godbolt.org/z/AjBHYqv which does a good job showing what happens when trying to compare an integer value and float value. See the precision loss:

Yes, conversion of numeric_limits<long long>::max to double rounds to a value out of range for long long. That's not what I'm talking about. Very specifically, in this line:
if (__r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY))
_MaxVal, by construction, is representable both as _RealT and as _IntT, so the static_cast does not change the value (so the rounding demonstrated in your godbolt link doesn't create a bug). a >= nextafter(b, INFINITY) is equivalent to a > b for any finite floatingpoint a and b. So this condition can simply be if (__r > static_cast<_RealT>(_MaxVal)).
Yes, that sounds right. I can't think of any reason that the condition couldn't be if (__r > static_cast<floatT>(numeric_limits<intT>::max())). The information lost from shifting the value around is never more than the information lost from static_casting the value (as far as I have been able to reason and test).
test/libcxx/numerics/truncating_cast.pass.cpp  

12 ↗  (On Diff #217513)  closes > closest 
Address review comments.
 Document that NaN isn't a supported input.
 Fix spelling mistake.
Taking over because Eric is on vacation. I think everything has been addressed at this point.
@scanon do you see anything else that needs to change?
I believe that the code can still be simplified somewhat, but that it's correct asis for float, double, and long double. I'll take an AI to followup on future improvements, and let's get this in.