nick8325 / quickcheck

Automatic testing of Haskell programs.
Other
724 stars 121 forks source link

Arbitrary instance for CUSeconds doesn't work on x86_32 with QuickCheck >= 2.10.1 #201

Closed arybczak closed 6 years ago

arybczak commented 6 years ago

Consider the following:

module Main where

import Foreign.C.Types
import Test.QuickCheck

prop_eq :: CUSeconds -> Bool
prop_eq x = x == x

main :: IO ()
main = quickCheck prop_eq

When compiled on multilib x86_64 Gentoo Linux with 64bit GHC-8.0.2 and QuickCheck-2.11.3, the test passes:

+++ OK, passed 100 tests.

However, when compiled with 32bit GHC-8.0.2 and QuickCheck-2.11.3, it fails:

*** Failed! Exception while generating shrink-list: 'Enum.fromEnum{Word32}: value (4294967295) is outside of I
nt's bounds (-2147483648,2147483647)' (after 1 test):                                                        
Exception thrown while showing test case: 'Enum.fromEnum{Word32}: value (4294967295) is outside of Int's bound
s (-2147483648,2147483647)' 

Note that QuickCheck-2.10.0.1 works fine with both 32bit and 64bit GHC versions. It seems that Arbitrary instance for CUSeconds was broken in 2.10.1.

nick8325 commented 6 years ago

Oh, interesting! We use fromEnum on CUSeconds, but this is actually a partial function on 32-bit GHC (because a CUSeconds is really a Word32 which does not fit in an Int), hence the crash.

Not immediately sure what to do about this, but it should be fixable.

The change in question was made to support GHC 7.2, which defines CUSeconds but doesn't export its constructor, meaning that we have to write the Arbitrary instance in an indirect way. One option would be just to drop this support and change back to the previous Arbitrary instance.

arybczak commented 6 years ago

I'm all for dropping support for GHC 7.2, it's 6 years old.

sol commented 6 years ago

The change in question was made to support GHC 7.2

If this is for 7.2 specifically, then I think there is no big benefit in having this. 7.2 was a tech preview which is/was not widely used for real world stuff. If it's also for 7.0.* then that is a separate consideration.

@nick8325 I assume this is not what you suggested, but for me it would be fine to drop support for 7.2.* and older all together. What I care about is that >= 7.4.1 works, that is what hspec still supports (AFAIK same for tasty).

nick8325 commented 6 years ago

7.2 was a tech preview which is/was not widely used for real world stuff.

Ah, I'd forgotten about that. In that case I agree it makes sense to drop support for 7.2 altogether.