D3316 status

Richard Eisenberg rae at cs.brynmawr.edu
Thu Mar 30 01:16:42 UTC 2017


> On Mar 29, 2017, at 2:31 PM, Ben Gamari <ben at smart-cactus.org> wrote:
> 
> Hi Richard,
> 
> It took me a bit longer than expected to get back to D3316 but I think I
> am now done. It wasn't quite as trivial as I thought it might be, so I
> thought I would summarize what was needed,
> 
> 1. tcSplitTyConApp_maybe and tcRepSplitTyConApp_maybe have been moved
>    from TcType to Type to avoid import loops. I'm not terribly happy
>    with this change, but I spent a fair amount of time looking for
>    alternatives and it seems this is the least evil. Moreover, there is
>    precedent for this in tcRepSplitAppTy_maybe. It would be nice to
>    refactor TcType and friends to avoid this loop.

Why do this functions get involved? Where do they get called? Unify? If so, good catch!

> 
> 2. TypeMap now respects the Constraint/Type distinction. I believe this
>    is safe for the TypeMap uses in the typechecker itself. The only
>    other use AFAICT is Specialise.CallKeySet, which I believe should
>    also be fine under this change.

Another good catch.

> 
> 3. TcInteract.matchTypeable now handles Constraint explicitly.

Hooray.

> 
> With all of this the T11715 test passes. I've uploaded my changes
> relative to your patch as D3396 for your review.

Thanks!! Will look tomorrow.

> 
> However, I am a bit concerned with (2) as it runs contrary to your
> original implementation, which changed coreViewOneStarKind to coreView.
> What was the reasoning behind this?

The reasoning was that we should consider (instance C Constraint) and (instance C Type) to be overlapping, so any kind of lookup should be over coreView types. This was before the T11480b bug was known and before the decision to let these instances not overlap. So I agree with your decision for (2).

> 
> The motivation for moving to tcView here is the dictionary cache, which
> uses a TypeMap. Namely, if we solve for `Typeable Type`, and later look
> for `Typeable Constraint` in the dictionary cache, we will incorrectly
> find the `Typeable Type` dictionary.
> 
> Thoughts?

That you've hit this one on the head. :)

Richard

> 
> Cheers,
> 
> - Ben



More information about the ghc-devs mailing list