Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#11062 closed bug (fixed)

Type families + hs-boot files = panic (type family consistency check too early)

Reported by: goldfire Owned by:
Priority: high Milestone: 8.2.1
Component: Compiler Version: 7.11
Keywords: TypeFamilies hs-boot Cc: ezyang
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D2859
Wiki Page:

Description

I have these modules:

A.hs-boot:

{-# LANGUAGE TypeFamilies #-}
module A where
type family F a

B.hs:

{-# LANGUAGE TypeFamilies #-}
module B where
import {-# SOURCE #-} A
type instance F Int = Bool

A.hs:

{-# LANGUAGE TypeFamilies #-}
module A where
import B
type family F a

And now this happens:

rae:11:50:47 ~/temp/bug> ~/Documents/ghc-cur/inplace/bin/ghc-stage2 --version
The Glorious Glasgow Haskell Compilation System, version 7.11.20151104
rae:11:50:50 ~/temp/bug> ~/Documents/ghc-cur/inplace/bin/ghc-stage2 -c A.hs-boot
rae:11:50:52 ~/temp/bug> ~/Documents/ghc-cur/inplace/bin/ghc-stage2 -c B.hs
rae:11:50:54 ~/temp/bug> ~/Documents/ghc-cur/inplace/bin/ghc-stage2 -c A.hs
ghc-stage2: panic! (the 'impossible' happened)
  (GHC version 7.11.20151104 for x86_64-apple-darwin):
	tcIfaceGlobal (local): not found:
  F
  []

Please report this as a GHC bug:  http://www.haskell.org/ghc/reportabug

rae:11:50:57 ~/temp/bug> 

The problem occurs only in one-shot mode.

Attachments (1)

T11062.zip (663 bytes) - added by goldfire 4 years ago.
Test case files

Download all attachments as: .zip

Change History (18)

Changed 4 years ago by goldfire

Attachment: T11062.zip added

Test case files

comment:1 Changed 4 years ago by ezyang

Oddly enough this is a regression from GHC 7.6.3

comment:2 Changed 4 years ago by ezyang

Cc: ezyang added

comment:3 Changed 4 years ago by thomie

Keywords: TypeFamilies added

comment:4 Changed 4 years ago by bgamari

Milestone: 8.0.18.2.1

It doesn't appear that this will get fixed for 8.0.1.

comment:5 Changed 3 years ago by ezyang

Another example of this is T6018, when built in one-shot mode.

comment:6 Changed 3 years ago by ezyang

Keywords: hs-boot added

comment:7 Changed 3 years ago by ezyang

OK, it's actually pretty obvious what's going on here: family instance consistency gets checked in the renamer but this is too early, because we haven't put enough things in the type environment. The solution is to move the check later in the typechecking process.

comment:8 Changed 3 years ago by ezyang

Differential Rev(s): Phab:D2215
Status: newpatch

If it is completely unacceptable to check type family instance consistency after type checking, then we'll have to look into a different way of fixing this problem.

comment:9 Changed 3 years ago by simonpj

I'm going to respond here to the comments on Phab:D2215 because I think Trac comments are more likely to survive long term than Phab discussion.

The question is: where should we check for consistency of imported type-family axioms?

Currently it's done right after processing the import declarations of a module; but that's too early when we are concerned about consistency of axioms for a type family declared in this module.

The patch does it late, after type checking all the local declarations. That's bad too, because all that type checking could be done with bogus overlapping family instances.

I can think of two solutions.

  1. Check for consistency of axioms for F right after F is defined. That is, somewhere in TcTyClsDecls.tcTyClGroup. Notes:
    • We'd still need to check consistency for imported families before we start typechecking anything.
    • With this more incremental behaviour it might be harder to optimise the number of pairs compared (see checkFamInstConsistency module-pair stuff). But that doesn't matter since it is vanishingly rare for a type family defined in this module to have any imported instances whatsoever.
  1. Make family-instance lookup complain if it finds two matching instances.
    • That would means we'd never make use of inconsistent instances, which would prevent strange error messages
    • We'd still need to do an aggressive checkFamInstConsistency check after type checking, to ensure consistency of instances that we didn't actually need when compiling this module (see the comments above FamInst.checkFamInstConsistency.

I'm not sure which is best. I think (2) looks a bit easier, and is quite close to the proposed patch; just needs an overlap check on lookup.

comment:10 Changed 3 years ago by simonpj

Another minor advantage of (2) is that we could remove the

       ; no_conflict    <- checkForConflicts inst_envs fam_inst

check in FamINst.addLocalFamInsts, because inconsistency would be caught by the later aggressive check, and would not cause a problem meanwhile.

comment:11 Changed 3 years ago by ezyang

I attempted to do (2) but actually it is not that easy. The difficulty stems from the fact that some of the functions that query FamInstEnv, e.g., normaliseType :: FamInstEnvs -> Role -> Type -> (Coercion, Type) never fail; that is, they assume that there are no conflicting instances. So it would seem all of these functions would need to have their signatures changed that they might fail due to a conflicting instance. Might be worth it, but certainly not a non-invasive change.

comment:12 Changed 3 years ago by ezyang

Summary: Type families + hs-boot files = panicType families + hs-boot files = panic (type family consistency check too early)

comment:13 Changed 3 years ago by ezyang

There is a technical difficulty with (1): we need to be able to distinguish between imported families and families which we are going to define. Unfortunately, these families are all bundled together in a FamInstEnv, and the only indication if it's local or external is in the Name of the CoAxiom, but we're not allowed to actually poke the CoAxiom to get this Name (since that will force the thunk.) So we need to carry along the Name of the FamInstEnv, or separate out local/external while we're typechecking the interface. The latter sounds less intrusive; we'll just end up with two FamInstEnvs in this case.

EDIT: Blast! This is nonsense: it's not the instance that's the problem, it's the type family.

Last edited 3 years ago by ezyang (previous) (diff)

comment:14 Changed 3 years ago by ezyang

Differential Rev(s): Phab:D2215Phab:D2859

OK, we've got a new patch!

comment:15 Changed 3 years ago by Edward Z. Yang <ezyang@…>

In 25b70a29/ghc:

Check family instance consistency of hs-boot families later, fixes #11062.

Summary:
With hs-boot files, some type families may be defined in the
module we are typechecking.  In this case, we are not allowed
to poke these families until after we typecheck our local
declarations.  So we first check everything involving non-recursive
families, and then check the recursive families as we finish
kind-checking them.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: goldfire, austin, simonpj, bgamari

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2859

GHC Trac Issues: #11062

comment:16 Changed 3 years ago by ezyang

Resolution: fixed
Status: patchclosed

comment:17 Changed 3 years ago by ezyang

See #13251 for a follow-up.

Note: See TracTickets for help on using tickets.