Opened 6 years ago

Closed 2 years ago

#7437 closed bug (fixed)

peculiar behaviour with default instances and type variables

Reported by: bos Owned by: simonpj
Priority: normal Milestone:
Component: Test Suite Version: 7.6.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC accepts invalid program Test Case: typecheck/should_fail/T7437
Blocked By: Blocking:
Related Tickets: #12151 Differential Rev(s):
Wiki Page:

Description

Here is a small module that has perplexing behaviour under GHC 7.6.1.

In its current form, it compiles and behaves correctly.

There are two lines commented out in the module.

Uncomment the first type signature for "default put", comment out the second one, and recompile. You'll get an "ambiguous constraint" error.

Now uncomment the FlexibleInstances pragma. The error changes to "no instance for (Put a0)", and moves to the bottom of the file.

I encountered this problem earlier today, in a module (in the binary package) that had FlexibleInstances and a default signature where the name of the type variable did not match the name of the class's type variable.

In that module, I did not have an instance like the one at the bottom of the file, so the module compiled cleanly and without error.

It was not until I tried to compile the module that depended on it that the error occurred. It took me three hours of poking around at random before I accidentally figured out what was wrong.

It would be very helpful if GHC either accepted default signatures with non-matching type variables (which seems correct to me) or rejected them, but did one or the other consistently. Finding a mysterious type error in a downstream module because of a very hard-to-spot error in an upstream module turned out to be extremely difficult.

{-# LANGUAGE DefaultSignatures, FlexibleContexts, DeriveGeneric #-}
-- {-# LANGUAGE FlexibleInstances #-}

module Whee where

import GHC.Generics

class GPut f where
    gput :: f a -> [()]

class Put a where
    put :: a -> [()]

    -- default put :: (Generic t, GPut (Rep t)) => t -> [()]
    default put :: (Generic a, GPut (Rep a)) => a -> [()]
    put = gput . from

instance GPut U1 where
    gput U1 = []

instance GPut a => GPut (M1 i c a) where
    gput = gput . unM1

data Foo = Foo
         deriving (Generic)

instance Put Foo

Change History (11)

comment:1 Changed 6 years ago by simonpj

difficulty: Unknown

Here is what is going on. When you replace the second sig for default put with the first, the type of the generic default method becomes

  $gdmput :: forall a. Put a => forall t. (Generic t, GPut (Rep t)) => t -> [()]

and this is indeed ambiguous. But the error message is unhelpful

T7437.hs:11:1:
    Ambiguous constraint `Put a'
      At least one of the forall'd type variables mentioned by the constraint
      must be reachable from the type after the '=>'
    When checking the class method: put :: a -> [()]
    In the class declaration for `Put'

Second, as things stand when you add FlexibleInstances that suppresses the ambiguity check (see Note [The ambiguity check for type signatures] in TcMType.lhs). So instetad you get ambiguity where the generic defaul method is called. The instance Put Foo really means

instance Put Foo where
  put = $gdmput

and so you get

T7437.hs:27:10:
    No instance for (Put a0) arising from a use of `T7437.$gdmput'
    The type variable `a0' is ambiguous
    Possible fix: add a type signature that fixes these type variable(s)
    Note: there is a potential instance available:
      instance Put Foo -- Defined at T7437.hs:27:10
    In the expression: (T7437.$gdmput)
    In an equation for `put': put = (T7437.$gdmput)
    In the instance declaration for `Put Foo'

Things for me (or someone) to think about

  • Improve the context info for the first message, which should at least give the ambiguous type.
  • Improve the ambiguity check, which is very crude at the moment. The ambiguity check that is used for infered types is this. Given f = e in you infer the type f :: type, then would this type check?
      g :: type
      g = f
    
    The ambiguity check for declared type should be the same, and it isn't.

So this does not look urgent but clear rooom for improvement. Thanks for reporting.

comment:2 Changed 6 years ago by igloo

Milestone: 7.8.1
Owner: set to simonpj

comment:3 Changed 4 years ago by thoughtpolice

Milestone: 7.8.37.10.1

Moving to 7.10.1

comment:4 Changed 4 years ago by thomie

Component: CompilerCompiler (Type checker)
Status: newinfoneeded

Uncommenting the FlexibleInstances pragma does not have an effect on the error message with ghc HEAD (or ghc-7.8.3), and the message also seems much improved:

$ cat test.hs
{-# LANGUAGE DefaultSignatures, FlexibleContexts, DeriveGeneric #-}
{-# LANGUAGE FlexibleInstances #-}

module Whee where
...

$ ghc-7.9.20141121 test.hs
[1 of 1] Compiling Whee             ( test.hs, test.o )

test.hs:12:1:
    Could not deduce (Put a0)
    from the context (Put a, Generic t, GPut (Rep t))
      bound by the type signature for
                 put :: (Put a, Generic t, GPut (Rep t)) => t -> [()]
      at test.hs:(12,1)-(17,21)
    The type variable ‘a0’ is ambiguous
    In the ambiguity check for the type signature for ‘put’:
      put :: forall a.
             Put a =>
             forall t. (Generic t, GPut (Rep t)) => t -> [()]
    To defer the ambiguity check to use sites, enable AllowAmbiguousTypes
    When checking the class method: put :: a -> [()]
    In the class declaration for ‘Put’

Does that means this issue is resolved?

comment:5 Changed 4 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:6 Changed 3 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:7 Changed 3 years ago by thomie

Component: Compiler (Type checker)Test Suite
Status: infoneedednew

Bug is fixed. Just needs a test.

comment:8 Changed 3 years ago by thomie

Milestone: 8.0.1

comment:9 Changed 2 years ago by thomie

I wished I had added that test already, because now this issue has regressed. See #12151.

comment:10 Changed 2 years ago by simonpj

In 85aa6ef0/ghc:

Check generic-default method for ambiguity

Fixes Trac #7497 and #12151.   In some earlier upheaval I introduced
a bug in the ambiguity check for genreric-default method.

This patch fixes it.  But in fixing it I realised that the
sourc-location of any such error message was bogus, so I fixed
that too, which involved a slightly wider change; see the
comments with TcMethInfo.

The commit message has a typo about the ticket.

Last edited 2 years ago by simonpj (previous) (diff)

comment:11 Changed 2 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: typecheck/should_fail/T7437
Note: See TracTickets for help on using tickets.