Use Integer in towards for halving during shrinking

[jonfowler]

This fixes the odd situation in which 2 is not representable as a number in a type. In particular this occurs in the package 'clash-prelude', which defines types such as Unsigned 1 where only 0 and 1 can be represented.

This is in a sense more of an issue with clash types not having fully valid Num instances, however it is very easy to fix here and having an error during shrinking is very difficult to debug!

[moodmosaic]

This fixes the odd situation in which 2 is not representable as a number in a type. In particular this occurs in the package clash-prelude, which defines types such as Unsigned 1 where only 0 and 1 can be represented.

This looks reasonable, though I was wondering if we can tell the bounds of the type instead of toInteger ?

[jonfowler]

I don't think so - not without adding extra type class constraints (and there is the inevitable problem that Integer itself has no bounds)

[jacobstanley]

I think you detect it by roundtripping 2 through fromInteger/toInteger, the question is if GHC is smart enough to make that a constant.

[jacobstanley]

@jonfowler have you got an example usage?

one possibility is to use Gen.choice with the two options

It is bad that it was hard to debug though, so it would be good to fix

[jonfowler]

@jacobstanley this is the example:

{-# LANGUAGE DataKinds #-}

-- clash-prelude
import Clash.Prelude

-- hedgehog
import Hedgehog
import qualified Hedgehog.Range as Range
import qualified Hedgehog.Gen as Gen

-- This fails 50% of the time with:
-- @
-- *Main> failCase
-- === Outcome ===
-- 1
-- === Shrinks ===
-- *** Exception: divide by zero
-- @
failCase :: IO ()
failCase
  = Gen.print $ Gen.resize 99 (Gen.integral Range.linearBounded :: Gen (Unsigned 1))

Obviously we can use enumBounded here, but considering this is a relatively easy fix it seems worthwhile? I can't see any downsides to the change (apart from doing a few more operations, which should be negligible).

[jonfowler]

Also this aligns the shrinking with how scaleLinear from Range works which also use Integer, otherwise we would see this type of issue generating the values as well.

[jonfowler]

@jacobstanley @moodmosaic can we merge this? I don't feel it is controversial (and I would prefer not to have to use a patched version).

If it is not going to get merged I will close rather than having it sit here.

[moodmosaic]

🏓 @jacobstanley

[jacobstanley]

I don't feel it is controversial

I do consider it controversial due to the use of Integer in a key function which is called recursively. I worry about the performance impact it will have without benchmarking.

I experimented with a few ideas and found a solution which I'm happy to merge, I'll modify this PR and push it through.

towards :: Integral a => a -> a -> [a]
towards destination x =
  if destination == x then
    []
  -- special case for 1-bit numbers
  else if destination == 0 && x == 1 then
    [0]
  else
    -- ... snip ...