Make use of boot TyThings during typechecking.
ClosedPublic

Authored by ezyang on Nov 5 2017, 2:06 PM.

Details

Summary

Suppose that you are typechecking A.hs, which transitively imports,
via B.hs, A.hs-boot. When we poke on B.hs and discover that it
has a reference to a type from A, what TyThing should we wire
it up with? Clearly, if we have already typechecked A, we
should use the most up-to-date TyThing: the one we freshly
generated when we typechecked A. But what if we haven't typechecked
it yet?

For the longest time, GHC adopted the policy that this was
*an error condition*; that you MUST NEVER poke on B.hs's reference
to a thing defined in A.hs until A.hs has gotten around to checking
this. However, actually ensuring this is the case has proven
to be a bug farm. The problem was especially poignant with
type family consistency checks, which eagerly happen before
any typechecking takes place.

This patch takes a different strategy: if we ever try to access
an entity from A which doesn't exist, we just fall back on the
definition of A from the hs-boot file. This means that you may
end up with a mix of A.hs and A.hs-boot TyThings during the
course of typechecking.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

Test Plan

validate

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ezyang created this revision.Nov 5 2017, 2:06 PM

Delete lots of tricky code. Make programs work that previously failed in hard-to-debug ways. What's not to like :-)?

Does it validate?

bgamari requested changes to this revision.Nov 6 2017, 1:59 PM

It seems like there should be more Note updates in this patch. Specifically, perhaps we should document precisely *when* we expect to need to fall back on hs-boot things? Moreover, it's not at all clear to me what we are trading off in this decision. What were the reasons for attempting to enforce the previous invariant?

This revision now requires changes to proceed.Nov 6 2017, 1:59 PM
austin resigned from this revision.Nov 9 2017, 5:43 PM
ezyang updated this revision to Diff 14715.Nov 16 2017, 8:56 PM

validate clean

ezyang updated this revision to Diff 14717.Nov 17 2017, 12:12 AM

one more validate fix

What is the status of this, @ezyang? Have you and @simonpj agreed that this is a reasonable way forward?

There are a lot of unknowns with this patch (e.g., whether or not it will cause some subtle bugs because a TyThing is not wired up) but Simon and I are in agreement that it's worth a try. Unfortunately harbormaster is segfaulting right now but I know this patch validate(d) on OS X without any trouble.

Edward: can you list the tickets that might be fixed by this patch? I think there are quite a few.

This revision was automatically updated to reflect the committed changes.