Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7171 closed bug (fixed)

erroneous overlapping instances reported with FunDeps

Reported by: jwlato Owned by:
Priority: highest Milestone: 7.6.1
Component: Compiler (Type checker) Version: 7.6.1-rc1
Keywords: type classes, fundeps Cc: jwlato@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: typecheck/should_compile/T7171
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

When a superclass constraint has functional dependencies, in certain cases GHC-7.6 erroneously reports that duplicate instances are found.

file Foo.hs

{-# LANGUAGE FunctionalDependencies #-}
{-# LANGUAGE FlexibleInstances #-}

module Foo where

import Data.ByteString as B
import Data.Word

class Foo a b | a -> b

class (Foo a b) => Bar a b | a -> b

instance Foo [a] a
instance Bar [a] a
instance Foo ByteString Word8
instance Bar ByteString Word8

test :: Bar full item => full -> full
test inp = inp

-- this works
-- _test_x :: ByteString -> ByteString
-- _test_x = test


file Main.hs

module Main where

import Foo
import Data.ByteString

-- this works
-- test1 :: [Int] -> [Int]
-- test1 = test

-- this fails
test2 :: ByteString -> ByteString
test2 = test

ghc reports

Main.hs:12:9:
    Overlapping instances for Foo ByteString GHC.Word.Word8
      arising from a use of `test'
    Matching instances:
      instance Foo ByteString GHC.Word.Word8 -- Defined at Foo.hs:20:10
    There exists a (perhaps superclass) match:
    (The choice depends on the instantiation of `'
     To pick the first instance above, use -XIncoherentInstances
     when compiling the other instance declarations)
    In the expression: test
    In an equation for `test2': test2 = test

For this to manifest, I think that at least the following must be true:

  • a function with class constraints must be called from a separate module
  • the type class instance must be monomorphic in the fundep-to parameter

This example works in ghc-7.4.2.

Attachments (2)

Foo.hs (354 bytes) - added by jwlato 7 years ago.
class/instances
Main.hs (165 bytes) - added by jwlato 7 years ago.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by jwlato

Attachment: Foo.hs added

class/instances

Changed 7 years ago by jwlato

Attachment: Main.hs added

comment:1 Changed 7 years ago by simonpj

difficulty: Unknown

Thanks. Happily I believe this is already fixed -- at least your test case works for me; I think the fix was the one for #7128. And the symptoms look very much like #7150, which also works now.

If you can try with HEAD that'd be great.

Simon

comment:2 Changed 7 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: typecheck/should_compile/T7171

Re-open if you think this is still broken.

comment:3 Changed 7 years ago by jwlato

I can confirm my problem is fixed in HEAD, but not ghc-7.6.0.20120820. Hopefully it'll be merged soon. Thanks for looking at this so quickly!

comment:4 Changed 7 years ago by simonpj

That is very strange. The patches I thought were important have been merged. I'll build 7.6 and try wit that. It's great to have your small test case, thank you.

comment:5 Changed 7 years ago by simonpj

Milestone: 7.8.1
Priority: normalhighest
Resolution: fixed
Status: closednew

comment:6 Changed 7 years ago by simonpj

Milestone: 7.8.17.6.1

I think this is fixed by this HEAD patch, which I have now merged into 7.6:

commit 702f0db0dd4e85de445e5edb5e6294057ebe6792
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Wed Aug 22 18:03:12 2012 +0100

        Numerous small changes to the constraint solver
    
        The main thing is that we now keep unsolved Derived constraints in the
        wc_flats of a WantedConstraints, rather than discarding them each time.
        This actually fixes a poential (admittedly obscure) bug, when we currently
        discard a superclass constraint, and may never re-generate it, and may
        thereby miss a functional dependency.
    
        Instead, reportErrors filters out Derived constraints that we don't want
        to report.
    
        The other changes are all small refactorings following our walk-through.
    
    MERGED from commit 9c0a6bbb0194f65cd62e48936c0c00fc4888eef3 on HEAD

 compiler/typecheck/TcErrors.lhs   |   53 +++++++++-
 compiler/typecheck/TcHsSyn.lhs    |   50 ++++------
 compiler/typecheck/TcInteract.lhs |  192 +++++++++++++------------------------
 compiler/typecheck/TcRnTypes.lhs  |   42 ++++----
 compiler/typecheck/TcSMonad.lhs   |  137 +++++++++++++-------------
 compiler/typecheck/TcSimplify.lhs |   86 +----------------
 6 files changed, 228 insertions(+), 332 deletions(-)

Can you try now with the 7.6 branch when the next snapshot appears?

I'll add your example as a regression test (for HEAD).

Simon

comment:7 Changed 7 years ago by simonpj

Resolution: fixed
Status: newclosed

Closing meanwhile, since we're all done here.

This patch also should fix the same problem in Hets, #7050.

Simon

comment:8 Changed 7 years ago by jwlato

I am happy to confirm that this is fixed for me with ghc-7.6.0.20120822. I think you meant the Hets issue in #7150; ticket #7050 appears unrelated.

Anyway, thanks again for fixing this, I'm glad the test case was helpful.

Note: See TracTickets for help on using tickets.