[#677] Make fromIntegralNoOverflow safe(r)
TODO (pre-merge checklist):
-
Amend subject/description of commit 'Make fromIntegralOverflowing safe(r)'. We decided to revert most of it (but not all of it), and reverting is pretty awkward and time-consuming with all the fixup commits. -
Amend subject/description of commit '[#677 (closed)] Add doctests to Morley.Prelude.FromIntegral', removing the note about tests not running on CI.
Description
Problem (while at it): fromIntegral functions are defined in the root Prelude module. This confuses things somewhat wrt what is an export, and what is not.
Solution: move definitions to a separate module. Only use root Prelude module for re-exports.
Problem (main): fromIntegralNoOverflow fails under some conditions:
> fromIntegralNoOverflow @Integer @Natural (-1)
*** Exception: arithmetic underflow
Solution: Catch the exception by forcing the conversion result to WHNF
inside unsafePerformIO
. For consistency, the return type was
changed to Either ArithException b
(from Either Text b
). Hence,
now fromIntegralNoOverflow
can be moved outside of Unsafe
.
However, moving the definition creates awkward circular dependencies
between Unsafe
and FromIntegral
, due to unsafe fromInteger
.
Hence, fromInteger
is moved to Unsafe
, and is re-exported from
Prelude
for compatibility considerations. Re-exporting from
Unsafe
in Prelude feels weird, but the alternatives I could
come up with are worse.
Design notes: Consider, for instance, this convoluted contraption:
fromIntegralNoOverflow @Natural @Int (negate 1)
The conversion is fine, but the argument is a bottom. Hence, to avoid
catching and masking this Underflow
exception, we need to force the
argument before doing our whole unsafe IO performance.
Related issue(s)
Resolves #677 (closed)
✅ Checklist for your Merge Request
Related changes (conditional)
-
Tests (see short guidelines)
-
If I added new functionality, I added tests covering it. -
If I fixed a bug, I added a regression test to prevent the bug from silently reappearing again.
-
-
Documentation
Stylistic guide (mandatory)
-
My commits comply with the following policy. -
My code complies with the style guide.