zoecarver added a comment.
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).
================
Comment at: include/math.h:1556
+_LIBCPP_BEGIN_NAMESPACE_STD
+template <class _IntT, class _FloatT,
----------------
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.
================
Comment at: include/math.h:1558
+template <class _IntT, class _FloatT,
+ bool _FloatBigger = (numeric_limits<_FloatT>::digits >
numeric_limits<_IntT>::digits)>
+struct __max_representable_int_for_float;
----------------
Nit: maybe qualify all the uses of `numeric_limits` and similar?
================
Comment at: include/math.h:1572
+ static_assert(is_integral<_IntT>::value, "must be an integral type");
+ enum { _Bits = numeric_limits<_IntT>::digits -
numeric_limits<_FloatT>::digits };
+ static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;
----------------
What is the enum providing for you? Couldn't this just be `static const int
_Bits = ...`?
================
Comment at: include/math.h:1573
+ enum { _Bits = numeric_limits<_IntT>::digits -
numeric_limits<_FloatT>::digits };
+ static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;
+};
----------------
What's the reasoning behind shifting something forward and back? Shouldn't this
always negate the other operation?
================
Comment at: include/math.h:1579
+_IntT __truncating_cast(_RealT __r) _NOEXCEPT {
+ using _Lim = std::numeric_limits<_IntT>;
+ const _IntT _MaxVal = __max_representable_int_for_float<_IntT,
_RealT>::value;
----------------
This will not work before C++11.
================
Comment at: include/math.h:1582
+ const _RealT __trunc_r = __builtin_trunc(__r);
+ if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+ return _Lim::max();
----------------
Maybe change `INFINITY` to `std::numeric_limits< _RealT >::infinity()`
================
Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:10
+// __truncating_cast<IntT>(RealT)
+
+// Test the conversion function that truncates floating point types to the
----------------
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`
================
Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:25
+ struct {
+ double Input;
+ IntT Expect;
----------------
Maybe test with more than just `double`. `float`, `long double`, others?
================
Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:28
+ bool IsRepresentable;
+ } TestCases[] = {
+ {0, 0, true},
----------------
C++03 will not like this :P
Repository:
rCXX libc++
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66836/new/
https://reviews.llvm.org/D66836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
