Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4981 closed bug (fixed)

inconsistent class requirements with TypeFamilies and FlexibleContexts

Reported by: ganesh Owned by: simonpj
Priority: high Milestone: 7.2.1
Component: Compiler (Type checker) Version: 7.0.1
Keywords: Cc: ganesh@…, dimitris@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: indexed-types/should_compile/T4981-V1.hs, T4981-V2.hs, T4981-V3.hs
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

If I build the code below with -DVER=2, I get a complaint about PatchInspect (PrimOf p) being missing from the context of cleverNamedResolve.

This doesn't happen with -DVER=1 or -DVER=3

I presume that type class resolution is operating slightly differently in the different cases, but it's quite confusing - in the original code joinPatches did something useful and I was trying to inline the known instance definition. I would have expected it to be consistent between all three cases, either requiring the context or not.

{-# LANGUAGE CPP, TypeFamilies, FlexibleContexts #-}
module Class ( cleverNamedResolve ) where

data FL p = FL p

class PatchInspect p where
instance PatchInspect p => PatchInspect (FL p) where

type family PrimOf p
type instance PrimOf (FL p) = PrimOf p

data WithName prim = WithName prim

instance PatchInspect prim => PatchInspect (WithName prim) where

class (PatchInspect (PrimOf p)) => Conflict p where
    resolveConflicts :: p -> PrimOf p

instance Conflict p => Conflict (FL p) where
    resolveConflicts = undefined

type family OnPrim p

#if VER==1
class FromPrims p where

instance FromPrims (FL p) where

joinPatches :: FromPrims p => p -> p
#else
#if VER==2
joinPatches :: FL p -> FL p
#else
joinPatches :: p -> p
#endif
#endif

joinPatches = id

cleverNamedResolve :: (Conflict (OnPrim p)
                      ,PrimOf (OnPrim p) ~ WithName (PrimOf p))
                   => FL (OnPrim p) -> WithName (PrimOf p)
cleverNamedResolve = resolveConflicts . joinPatches

Change History (8)

comment:1 Changed 9 years ago by dimitris

Cc: dimitris@… added

comment:2 Changed 9 years ago by dimitris

Component: CompilerCompiler (Type checker)

Right, I know what's going on. The problem we are solving is:

  [Given] Conflict (OnPrim p),
     + its superclass: [Given] (PatchInspect (PrimOf (FL (OnPrim p))))
  [Given]  PrimOf (OnPrim p) ~ WithName (PrimOf p) 
  [Wanted] Conflict (FL (OnPrim p)) 

Now, we apply the instance to get new work:

  [Wanted] Conflict (OnPrim p) 
  [Wanted] PatchInspect (PrimOf (FL (OnPrim p)))

The second guy is the "silent parameter" wanted. Now, the Conflict wanted can be readily discharged so let's not worry about him. However:

  [Wanted] PatchInspect (PrimOf (FL (OnPrim p))) ~~> 
                 [Wanted] PatchInspect (PrimOf (OnPrim p)) 

Now, this could be readily discharged by the given but instead what happens is that it gets rewritten with the given equality to:

       [Wanted] PatchInspect (WithName (PrimOf p))

Then the instance triggers, and we get:

       [Wanted] PatchInspect (PrimOf p) 

which is the error we see.

So indeed this program is in muddy territory where givens (superclasses of givens, actually!) overlap with top-level instances.

I know why GHC is not picking the given up: it has to do with the fact that we have not saturated all possible equalities before we look for instances, but luckily this is something Simon and I are planning to fix pretty soon. The other thing that we should also do is remove these silent superclass parameters -- they used to exist for reasons related to recursive dictionaries but they are not necessary any more the way we have dealt with recursive dictionaries at the end.

comment:3 Changed 9 years ago by simonpj

  • Removing this silent superclass parameters is reasonable, and would suppress this particular error, but the underlying problem would remain.
  • Giving priority to equalities is also probably reasonable for efficiency reasons, and would also suppress this error, but again it's not a solution; for example, the equality in question might be somehow hidden inside a superclass equality of a class constraint that arises from a use of an instance...
  • I think the real solution is to refrain from using an instance declaration if there is an overlap with a Given. This is what we do already for overlapping instances (unless you use -XAllowIncoherentInstances); we just need to extend it for givens.

I'll discuss this with D.

comment:4 Changed 9 years ago by igloo

Milestone: 7.2.1
Priority: normalhigh

comment:5 Changed 9 years ago by igloo

Owner: set to simonpj

comment:6 Changed 9 years ago by dimitris

OK I agree with Simon but the saturation of equalities is independently useful and I have already implemented it and pushed, so this example works now. I will add it to the testsuite and start implementing Simon's proposal.

comment:7 Changed 9 years ago by dimitris

Resolution: fixed
Status: newclosed

comment:8 Changed 9 years ago by dimitris

Test Case: indexed-types/should_compile/T4981-V1.hs, T4981-V2.hs, T4981-V3.hs
Note: See TracTickets for help on using tickets.