Change History (25)

Fix kind generalisation for pattern synonyms
We were failing to zonk, after quantifyTyVars, and that left
un-zonked type variables in the final PatSyn.
This fixes the patsyn/ problems in Trac #11648, but not
the polykinds/ ones.

This is wrong: the t_aSB should be instantiated to Lifted. It's terrible for a meta-tyvar to end up in the kind of a TyCon.

So I'm worried about where this defaulting happens in TcHsType.kcHsTyVarBndrs with cusk = True. Indeed, by adding tracing to this program I've confirmed that Monoidy also gets a kind (at least during kind-checking) looking like

But even if we fix that I'm worried about doing this CUSK stuff for associated types. I think it's probably wrong. We are crawling over the class decl to find constraints on the kind variables so we can infer the class decl's kind. We start that process with getInitialKinds; and we can't at that moment quantify over the kind variables we are about to gather constraints for!

At very least we must elaborate Note [Complete user-supplied kind signatures] to cover associated types. An associate type should have a CUSK iff (a) its parent class does, and (b) any private tyvars have kind sigs.

I'm a bit puzzled about why we call kcTyClDecl and generaliseTCDat all for decls with a CUSK.

Does that have a CUSK? According to the CUSK rules for datatypes, it does indeed: all of the type variables have a kind signature. But there's a problem in that we don't know k's type! So this kind signature isn't complete. Instead, we should need to say, e.g.,

data T :: forall (k :: *). Proxy k -> *

Note the explicit :: * on k's binder, which is necessary for us to have completely specify the kind.

However, in looking more closely at this case, I don't actually think this is the problem. A mistake in detecting CUSKness should affect type inference and whether or not we have principal kinds, etc., but it shouldn't produce the drastic failure that we're seeing above. Somehow, it seems that quantifyTyVars is making a mistake. I don't have the ability to test this at the moment (I'm in an intermediate state) but should get to this later this week. I propose to fix the immediate problem (which should be straightforward) and then to delay the debate about CUSKs for another time.

But quantifyTyVars is called from kindGeneralize within kcTyClGroup.generalise, no? I agree that, if our CUSK check were totally accurate, we should skip kcTyClDecl and generaliseTCD, but we currently don't. So quantifyTyVars should get called.

Ah -- so the meta-tyvars aren't making it to the real TyCon, only the TcTyCon? It's quite normal for TcTyCons to have meta-tyvars. (Indeed, that's what they're for.)

But I'm missing some context. The ASSERT that's breaking is the check during substitutions. But which substitution? I don't see an obvious substitution in getFamDeclInitialKind.

To be clear, I think there are two related problems going on:

The CUSK check is inaccurate in the presence of TypeInType, because implicitly-bound kind variables (like k in data Proxy (a :: k)) can have non-trivial types.

The code in TcTyClsDecls is tripping over CUSK types that aren't actually CUSKs.

I propose to fully defer (1). It can be triggered only with TypeInType enabled and has some thorny design issues. And when it goes wrong, it won't be catastrophic (i.e., no panicking, segfaulting, or launching rockets, just an unexpected -- but still sound -- result of type inference).

(2) is the real issue at hand. And, currently without the ability to test, I don't fully understand what's going wrong. I hope to be out of my intermediate state today or tomorrow.

Ah -- so the meta-tyvars aren't making it to the real TyCon, only the TcTyCon? It's quite normal for TcTyCons to have meta-tyvars. (Indeed, that's what they're for.)

Yes if they are a bare meta-tyvar, but not if they are full polymorphic kind gotten from a CUSK. The whole point about being "complete" is that the signature is fully defined. We experimented with partially-defined signatures and backed off. Complete means complete, and that means no lurking unification variables.

But I'm missing some context. The ASSERT that's breaking is the check during substitutions. But which substitution? I don't see an obvious substitution in getFamDeclInitialKind.

It happens during the immediately following tc_lhs_type; at an occurrence of MComp we find that it has a polymorphic kind and try to instantiate that kind. The instantiation fails with this error. (But it's just a canary in the mine; the real problem is that unification variable lurking in an allegedly-complete signature.

So yes, (2) is the issue at hand. To go back to your example:

data T (a :: Proxy k)

If we are to treat this as a CUSK (and I think we should) we must fix (at that very moment) the kind of k. Fixing it to * is fine, although its a kind-of-arbitrary choice. But fix it we must. I think the place may be in kcHsTyVarBndrs, which I do not fully understand.

We already had that the quantified type variable itself must not be a meta-tyvar. But this invariant goes one step further, in that the set of variables reachable from a quantified tyvar must be devoid of meta-tyvars. Do you agree with this invariant?

And, as for design, it seems you are proposing (forgetting about associated types for the moment):

Keep the syntactic definition of CUSK as it is (Note [Complete user-supplied kind signatures] in HsDecls).

When a declaration has a CUSK, default any remaining meta-tyvars after getInitialKind.

If a meta-tyvar is not defaultable, the program is erroneous.

I'm quite dubious of this design for several reasons. First, this seems strange to users. For example:

data T1 (a :: Proxy k)
data T2 (a :: Proxy k) b

T1 has a CUSK, according to our definition. Thus its k will be defaulted to kind *. T2, though, does not have a CUSK, and so k is not defaulted and has kind k2.

Second, the design seems hard to implement without annoying users. For example:

data T3 (x :: * -> *)
data T4 (a :: T3 k)

T4 has a CUSK, and k should clearly have kind * -> *. But we can't discover that until kcTyClDecl, which is too late. According to the last bullet above, we'll fail at defaulting T4's k's kind and issue an error, even though it's quite obvious what we should do.

So, I must ask: why is it so necessary to have the INVARIANT at the top? I understand this requires expanding the in-scope set for the substitution at hand, but why is that problematic? I don't see what's so bad about a meta-tyvar in a quantified variable's kind.

This code is intended to deal with open data/type family declarations, which always have a CUSK, by fiat. It shouldn't trigger in any other case, because all other declaration forms require KindedTyVars to make a CUSK.

For your T1/T2 example I'm not too bothered. CUSKs mean that the user has specified the complete kind. If you want k to have some other kind than * you can specify that too (I hope).

OK.

For T3/T4, note that kdHsTyVarBndrs calls TcHsTyVarBndr_Scoped which in turn calls unifyKind so I think you'll be fine. (BUt there is clearly a missing solveEqualities here!

Yes, I suppose that's true.

But I don't spot a missing solveEqualities anywhere. Did you miss the call toward the top of kcTyClGroup?

Bottom line here: I'm unconvinced about INVARIANT. But it does seem easy enough to implement "all declarations with CUSKs default all meta-tyvars", which solves the main problem here. And then we just punt on INVARIANT. In other words, I wish to address (1) from comment:14, which will then solve (2) on the way.

Suppose you instantiate f :: forall k. forall (a :: kappa). blah at some call site. Make up a fresh meta-var for k and substitute. We won't substitute for kappa because it isn't k.

Later we unify kappa := k. And now our earlier instantiation of f's type is utterly wrong.

Actually the real INVARIANT is probably this:

In any quantified type forall a. blah, all occurrences of a in blah must be manifest: not hidden inside meta-tyvars, especially if they have not been instantiated.

There could in principle be some free meta-tyvars that we somehow know can never be filled in with the quantified type variable. But for top-level types or kinds this is never necessary.

Anyway I agree with your plan. But

But I don't spot a missing solveEqualities anywhere. Did you miss the call toward the top of kcTyClGroup?

I'm still looking at kcHsTyVarBndrs. It builds a polytype with mkSpecForAllTys. It does unification (inside the call to tcHsTyVarBndr_Scoped). I think we need to solve the equalities before we bind the type variables that are mentioned, don't we? Ugh -- that kcHsTyVarBndrs is horribly complicated.

Fix #11648.
We now check that a CUSK is really a CUSK and issue an error if
it isn't. This also involves more solving and zonking in
kcHsTyVarBndrs, which was the outright bug reported in #11648.
Test cases: polykinds/T11648{,b}
This updates the haddock submodule.
[skip ci]

A lot of the changes are due to moving decisions out from kcHsTyVarBndrs, and thus the need to store information in the AST:

It turned out to be more convenient to decide if a data decl has a CUSK in the renamed than on the fly. (It could be done on the fly, but it would have been a bit hairy and duplicated code in the renamer.) See the new bit in the manual for a description of what's going on here.

There also needed to be a decision about whether to use Anon or Named when building the TcTyBinders of a tycon's kind. Previous to this patch, this was done in kcHsTyVarBndrs, but -- as Simon pointed out to me -- this was bogus (it looked at the free variables of a not-yet-solved-or-zonked type). So it's now done via a simple syntactic check in the renamer. But then the result of the check must be put in the AST, causing knock-on changes. See Note [Dependent LHsQTyVars] in TcHsType and also a new bit in the manual.

One of this issues brought up in the ticket is the handling of CUSKs for associated type families. An associated type/data family's CUSKness must be inherited from the enclosing class. Previously, all associated families had CUSKs, but this was wrong. So the CUSK check is a bit more intricate to allow for this relationship.

Along the way, I made (->), when used in a kind, have type * -> * -> *, instead of being polymorphic in RuntimeReps. We don't have type-level representation polymorphism.

Also incidental to this patch, I made kind variables have kind * when -XTypeInType is not in effect. Not doing this earlier was an oversight. The check isn't perfect, as it's sometimes hard to tell what's a kind variable and what isn't; currently, only variables used in kinds in tycon LHsQTyVars are affected, and these are surely kind variables. (This is mk_kv_kinds in kcHsTyVarBndrs.)

There is a new function anonymiseTyBinders. This was necessary in an earlier version of this patch but is now redundant (and harmless). I will remove.