Prevent std::ldexp underflowing/overflowing because of hard-coded flo… #1284
Conversation
…at type. With exceptions disabled, I would expect `errno` to be set only on real issues. However I was hit by errno set to ERRNO for following example: ``` T crossover = ldexp(1.0f, iround(99 / -0.654f)); ```
There was a problem hiding this comment.
This looks reasonable to me, and I kicked off the CI run. Any different opinion @jzmaddock?
Not sure why it got marked as requested changes instead of general comments. My mistake
|
I think we need a test case, because to my eyes this doesn't actually change anything? Note that locally I'm unable to reproduce, but that may be because whether ERANGE is set on ldexp underflow is implementation defined. Off the top of my head, I think we need something more like: |
I'll try to add test case tomorrow. The notable change is that new code does not cast T to float anymore. Double precision was enough to avoid ERANGE.
it safe to use min_exponent here? I can see some boost support for FLT_RADIX higher than 2, and min_exponent is min exp for Other issue is that limit for T is used, but |
|
Ah yes, or just replacing 1.0f with static_cast(1) would do it. And you're right, my suggestion would fall foul of decimal types, though technically this change just pushes the problem further into the long grass as df > 1021 would still underflow with double. |
Prevent
std::ldexpunderflowing/overflowing because of hard-coded float type.With exceptions disabled, I would expect
errnoto be set only on real issues. However I was hit by errno set toERANGEfor following example:This means that
floatis not able to fitcrossovereven fordfas small as 99.