Fix build issue under ghc 7.10-rc3

[adituv]

Add FlexibleContexts to Data.HaskellModule.Parse to remove error at line 48 (in derived type, in the constraint IsString [a]).

There is another build issue, but I believe it to be out of your control: dependencies fail as MonadCatchIO-transformers==0.3.1.0 has the upper bound base<4.8.

This appears to be the only change necessary for hawk to compile under ghc 7.10 (assuming you give cabal the --allow-newer flag. Unfortunately, I'm still struggling to install everything necessary for the test suite still, so can't give feedback there yet, or run the suite to check everything is still working fine.

[gelisam]

Thanks! I mean, I don't think this is the right fix, but thanks for bringing my attention to it.

Line 48 is a function without a type signature:

isModuleDecl (Left xs) = "module " `B.isPrefixOf` xs
isModuleDecl (Right xs) = "module " `isPrefixOf` xs

isPrefixOf is not one of the many functions whose type was generalized in 7.10, so the type of isModuleDecl has always been inferred to be

isModuleDecl :: (Eq a, IsString [a]) => Either B.ByteString [a] -> Bool

Which should have required FlexibleContexts in any version of ghc, so I don't understand how this ever compiled.

Since I am familiar with the source, I know that the second branch is supposed to be examining a String, not any [a]. So I think the proper fix would be to add a type signature:

isModuleDecl :: Either B.ByteString String -> Bool
isModuleDecl (Left xs) = "module " `B.isPrefixOf` xs
isModuleDecl (Right xs) = "module " `isPrefixOf` xs

[gelisam]

Also, did you have to use special versions of some of the dependencies? I'm trying out ghc-7.10-rc2 in order to test your change and mine, but stringsearch-0.3.6.5's build also complains about needing FlexibleContexts.

[adituv]

Since I am familiar with the source, I know that the second branch is supposed to be examining a String, not any [a]. So I think the proper fix would be to add a type signature:

In that case, yes, making the type specific is the correct fix.

Also, did you have to use special versions of some of the dependencies? I'm trying out ghc-7.10-rc2 in order to test your change and mine, but stringsearch-0.3.6.5's build also complains about needing FlexibleContexts.

Yes, I had to manually patch some of the dependencies:

• "stringsearch" Data.ByteString.Search.Internal.Utils - add FlexibleContexts. Not sure if type signature is possible here - it's complaining about a non-type-variable argument in MArray a0 Int m
• "ansi-wl-pprint" Text.PrettyPrint.ANSI.Leijen - add import Prelude hiding ((<$>)). Maybe not necessary for rc2; I think (<$>) was only added to Prelude in rc3.

[gelisam]

Adding FlexibleContext to stringsearch doesn't look like the right fix either. Let's look at a simplified version of the problematic function:

suffShifts :: S.ByteString -> UArray Int Int
suffShifts pat = runSTUArray (do
    ar <- newArray ...  -- the first error message occurs here
    let sufShift !idx   -- the FlexibleContext error message occurs here
            | idx == patEnd = return ar
            | otherwise     = ...
    sufShift 0)

The first error message complains about an ambiguity in the line which creates ar:

Data/ByteString/Search/Internal/Utils.hs:184:11:
    No instance for (MArray a1 Int (GHC.ST.ST s))
      arising from a use of ‘newArray’
    The type variable ‘a1’ is ambiguous
    Note: there are several potential instances:
      instance MArray (STUArray s) Int (GHC.ST.ST s)
        -- Defined in ‘Data.Array.Base’
      instance MArray (STArray s) e (GHC.ST.ST s)
        -- Defined in ‘Data.Array.Base’

I believe those two potential instances are, respectively, for an unboxed and a boxed array. The unboxed array will be faster, and since the call to newArray is inside a runSTUArray block, it seems clear that it's the STUArray version which was intended.

But I don't understand how adding FlexibleContexts could fix that error message. In fact, I don't understand why there is any ambiguity at all: runSTUArray expects an ST computation returning an STUArray. Shouldn't that fix the type of ar? Well, only if ar is the value which is returned from the block. The block ends with a call to sufShift, a helper function which lacks a type signature. That's probably the cause of the problem.

The definition of sufShift has another error message, the one which probably prompted you to add FlexibleContexts:

Non type-variable argument in the constraint: MArray a_a4JF Int m
    (Use FlexibleContexts to permit this)
    When checking that ‘sufShift’ has the inferred type
      sufShift :: forall (m :: * -> *).
                  MArray a0 Int m =>
                  Int -> m (a0 Int Int)

Since sufShift is the last action of the runSTUArray block, we know that m (a0 Int Int) is supposed to match ST s (STUArray s i e), so the concrete type of sufShift is:

sufShift :: Int -> ST s (STUArray s Int Int)

I can't add this type signature directly because the s must be the same as the one which was used to create ar. So instead of adding FlexibleContext, I add ScopedTypeVariables and I give a type to the runSTUArray block, thereby placing s into scope:

suffShifts :: S.ByteString -> UArray Int Int
suffShifts pat = runSTUArray go
  where
    go :: forall s. ST s (STUArray s Int Int)
    go = do
      ar <- newArray ...
      let sufShift :: Int -> ST s (STUArray s Int Int)
          sufShift !idx
              | idx == patEnd = return ar
              | otherwise     = ...
      sufShift 0

Is it also you who has submitted the FlexibleContext fix for stringsearch? I'd like to submit my counter-proposal there.

[adituv]

No, I sourced my patch from that issue.  You seem far more familiar with GHC extensions than me; I wish I had just opened an issue now.
It was indeed the second problem that caused me to believe the patch, though your fix seems to fix it far better.
On 21 Mar 2015 13:51, =?ISO-8859-1?Q?Samuel_G=E9lineau?= [email protected] wrote:Adding FlexibleContext to stringsearch doesn't look like the right fix either. Let's look at a simplified version of the problematic function:

suffShifts :: S.ByteString -> UArray Int Int
suffShifts pat = runSTUArray (do
ar <- newArray ... -- the first error message occurs here
let sufShift !idx -- the FlexibleContext error message occurs here
| idx == patEnd = return ar
| otherwise = ...
sufShift 0)

The first error message complains about an ambiguity in the line which creates ar:

Data/ByteString/Search/Internal/Utils.hs:184:11:
No instance for (MArray a1 Int (GHC.ST.ST s))
arising from a use of ‘newArray’
The type variable ‘a1’ is ambiguous
Note: there are several potential instances:
instance MArray (STUArray s) Int (GHC.ST.ST s)
-- Defined in ‘Data.Array.Base’
instance MArray (STArray s) e (GHC.ST.ST s)
-- Defined in ‘Data.Array.Base’

I believe those two potential instances are, respectively, for an unboxed and a boxed array. The unboxed array will be faster, and since the call to newArray is inside a runSTUArray block, it seems clear that it's the STUArray version which was intended.

But I don't understand how adding FlexibleContexts could fix that error message. In fact, I don't understand why there is any ambiguity at all: runSTUArray expects an ST computation returning an STUArray. Shouldn't that fix the type of ar? Well, only if ar is the value which is returned from the block. The block ends with a call to sufShift, a helper function which lacks a type signature. That's probably the cause of the problem.

The definition of sufShift has another error message, the one which probably prompted you to add FlexibleContexts:

Non type-variable argument in the constraint: MArray a_a4JF Int m
(Use FlexibleContexts to permit this)
When checking that ‘sufShift’ has the inferred type
sufShift :: forall (m :: * -> *).
MArray a0 Int m =>
Int -> m (a0 Int Int)

Since sufShift is the last action of the runSTUArray block, we know that m (a0 Int Int) is supposed to match ST s (STUArray s i e), so the concrete type of sufShift is:

sufShift :: Int -> ST s (STUArray s Int Int)

I can't add this type signature directly because the s must be the same as the one which was used to create ar. So instead of adding FlexibleContext, I add ScopedTypeVariables and I give a type to the runSTUArray block, thereby placing s into scope:

suffShifts :: S.ByteString -> UArray Int Int
suffShifts pat = runSTUArray go
where
go :: forall s. ST s (STUArray s Int Int)
go = do
ar <- newArray ...
let sufShift :: Int -> ST s (STUArray s Int Int)
sufShift !idx
| idx == patEnd = return ar
| otherwise = ...
sufShift 0

Is it also you who has submitted the FlexibleContext fix for stringsearch? I'd like to submit my counter-proposal there.

—Reply to this email directly or view it on GitHub.

[bgamari]

gelisam wrote (emphasis mine),

isPrefixOf is not one of the many functions whose type was generalized in 7.10, so the type of isModuleDecl has always been inferred to be,

isModuleDecl :: (Eq a, IsString [a]) => Either B.ByteString [a] -> Bool

Which should have required FlexibleContexts in any version of ghc, so I don't understand how this ever compiled.

The last sentence is where the reasoning breaks down. Before GHC 7.10 the compiler would happily accept terms of types which require extensions so long as you didn't actually write a type signature.

The change is that the compiler now requires FlexibleContexts and friends to be enabled regardless of whether the type is inferred or explicit.

[gelisam]

Thanks for the clarification, I was wondering what had changed.

[gelisam]

I finally managed to get Hawk to compile with ghc 7.10.1-rc2, so I'll close this. For future reference, my steps were:

1. either apply this patch or my type signature fix
2. cabal get stringsearch-0.3.6.5 (the most recent version at the time of writing, which doesn't compile with ghc 7.10)
3. either apply this patch or my type signature fix
4. cabal sandbox add-source ./stringsearch-0.3.6.5/
5. cabal get hsc2hs-0.67.20120610 (the most recent version at the time of writing, which segfaults when I install network-2.6.0.2)
6. find the ghcconfig.h associated with ghc 7.10 (mine was installed by brew at /usr/local/Cellar/ghc/7.10.1-rc2/lib/ghc-7.10.0.20150123/include/ghcconfig.h) and place it at ../../include/ghcconfig.h where hsc2hs's compilation step will look for it
7. cabal install hsc2hs and put the executable on your path (I had to rename /usr/local/bin/hsc2hs to prevent that segfaulting one from being used)
8. cabal install --allow-newer

[gelisam]

In order to also run the tests with ghc 7.10, I had to do the following extra steps.

1. cabal install hashable-1.2.3.1 --allow-newer (the most recent version at the time of writing is hashable-1.2.3.2, which uses typeRepFingerprint, which isn't yet available in ghc 7.10.1-rc2)
2. cabal install aeson, as usual.
3. cabal install --allow-newer --enable-tests
4. for some reason the above doesn't run the tests anymore, it only installs them, and running cabal test runs into this issue, so I have to run the test suite manually: ./dist/dist-sandbox-*/build/reference/reference