make sure eval_convert_to() do not terminate with super large number

[tchaikov]

this change is a follow-up of d51f2e9. it intends to address the exception thrown in a noexcept functon.

a minimal reproducer looks like

int main() {
    std::string s = "32767456456456456456545678943512357658768763546575675";
    boost::multiprecision::cpp_int num(s);
    std::cout << num.convert_to<float>() << std::endl;
}

since boost 1.79, the code above terminates like

Program returned: 139
Program stderr
terminate called after throwing an instance of 'boost::wrapexcept<std::domain_error>'
  what():  Error in function float_next<float>(float): Argument must be finite, but got inf
Program terminated with signal: SIGSEGV

because float_next_imp() throws boost::wrapexcept<std::domain_error> if the number is NAN of INF. and eval_convert_to() is marked as noexcept(boost::multiprecision::detail::is_arithmetic<R>::value && std::numeric_limits<R>::has_infinity),
but only overflow_error is ignored in the policy passed to float_next().

so, in this change, std::domain_error is ignored as well, so that num.convert_to<float>() returns a NaN in this case.

Refs #553

[tchaikov]

a reproducer can be found at https://godbolt.org/z/K5nsj5azT

[tchaikov]

hi @jzmaddock , could you help review this change? my colleague @nyh believes that an "inf" should be returned instead, as it was the behaviour in boost 1.78. what do you think?

[jzmaddock]

It looks like it's fine: but your parenthesis are in the wrong place around make_policy, should be (I hope):

*result = boost::math::float_next(*result, boost::math::policies::make_policy(boost::math::policies::overflow_error<boost::math::policies::ignore_error>(),                                                                                             boost::math::policies::domain_error<boost::math::policies::ignore_error>()));

[jzmaddock]

Never mind, I see I can edit, have changed thing round...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.1%. Comparing base (fe3054f) to head (ea78649).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop    #618     +/-   ##
=========================================
+ Coverage     94.1%   94.1%   +0.1%     
=========================================
  Files          274     274             
  Lines        26765   26766      +1     
=========================================
+ Hits         25162   25163      +1     
  Misses        1603    1603             

Files	Coverage Δ
include/boost/multiprecision/cpp_int/misc.hpp	91.5% <100.0%> (+0.1%)	⬆️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe3054f...ea78649. Read the comment docs.

[jzmaddock]

Failures should be corrected by boostorg/math#1130 once it runs...

[tchaikov]

Never mind, I see I can edit, have changed thing round...

thank you! squashed and repushed.

[nyh]

hi @jzmaddock , could you help review this change? my colleague @nyh believes that an "inf" should be returned instead, as it was the behaviour in boost 1.78. what do you think?

I stand by my belief that when converting an integer to floating-point, truncation should result in +Inf (or -Inf) and not NaN. Returning Inf in the case of truncation, and not NaN, is traditional, for example:

>>> print(1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.0)
inf

but even more importantly, this also this was the behaviour in older Boost before this bug happened, and application code may rely on this truncation behavior for its own user-visible truncation (as for example happened in the ScyllaDB project).

So although after this patch the conversion no longer crashes (good), it's still wrong in my view.

[jzmaddock]

My bad, you're quite correct, see boostorg/math#1132.

[nyh]

My bad, you're quite correct, see boostorg/math#1132.

Thanks. I'm not familiar with Boost's unit tests, but if you have them, I recommend that you add regression tests for convert_to() whose results are expected to be both +inf and -inf, because it's possible the details of what broke (float_nex()t? I didn't understand the details) would be different in the two cases. Although float_next(+inf) might perhaps be defined, what is float_next(-inf) supposed to return?

UPDATE: I read the convert_to code again, and it seems it works on the positive number, and always adds the sign at the end - so it will always be +Inf, not -Inf, at this stage. So I think the code is indeed fine, and you just need float_next(+Inf) to return +Inf. Or, perhaps not call this function in the case of Inf?