Skip to content

[#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

    • I checked whether I should update the docs and did so if necessary:
    • I updated changelog files of all affected packages released to Hackage if my changes are externally visible.

Stylistic guide (mandatory)

Edited by Nikolay Yakimov

Merge request reports