Opened 18 months ago

Closed 5 weeks ago

#11785 closed task (fixed)

Merge types and kinds in Template Haskell

Reported by: goldfire Owned by:
Priority: normal Milestone: 8.4.1
Component: Template Haskell Version: 8.1
Keywords: TypeInType Cc: RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D3751, Phab:D3854
Wiki Page:

Description

TcSplice handles kinds differently than types. Now that they are the same, it's probably best to rewrite this code.

Change History (10)

comment:1 Changed 18 months ago by RyanGlScott

Cc: RyanGlScott added

comment:2 Changed 4 months ago by RyanGlScott

Another place we should unify the treatment of Types and Kinds is DsMeta. We have both repLTy and repLKind, but the latter covers far fewer things than the former, which has led to bugs like #13781.

Sadly, accomplishing this requires more work than you'd believe, since repLTy returns a TypeQ, whereas repLKind returns a pure Kind. In other words, we'd have to go through and purify repLTy, as well as every function it calls which is also monadic (see https://phabricator.haskell.org/D3627#103117).

comment:3 Changed 4 months ago by goldfire

Keywords: TypeInType added

comment:4 Changed 2 months ago by RyanGlScott

Summary: Kinds should be treated like types in TcSpliceMerge types and kinds in Template Haskell

Actually, there's another way to go about this. Instead of making more things pure, we could make more things monadic. This can be accomplished with the following combinator:

bindCore :: DsM (Core (TH.Q a)) -> (Core a -> DsM (Core (TH.Q b)))
         -> DsM (Core (TH.Q b))
bindCore dsma f
  = do { cqa@(MkC qa) <- dsma
       ; loc <- getSrcSpanDs
       ; a_name <- newNameAt (mkVarOccFS (fsLit "a")) loc
       ; let [a_ty] = tcTyConAppArgs (exprType qa)
       ; let a_id = mkLocalId (localiseName a_name) a_ty
             ca   = MkC (Var a_id)
       ; MkC qb <- f ca
       ; let [b_ty] = tcTyConAppArgs (exprType qb)
       ; repBindQ a_ty b_ty cqa (MkC (Lam a_id qb)) }

With bindCore, we can retrofit existing uses of repLKind with repLTy. Here's one example:

  • compiler/deSugar/DsMeta.hs

    diff --git a/compiler/deSugar/DsMeta.hs b/compiler/deSugar/DsMeta.hs
    index c679981..8acc6eb 100644
    a b repTy (HsEqTy t1 t2) = do 
    10431044                         repTapps eq [t1', t2']
    10441045repTy (HsKindSig t k)       = do
    10451046                                t1 <- repLTy t
    1046                                 k1 <- repLKind k
    1047                                 repTSig t1 k1
     1047                                bindCore (repLTy k) $ repTSig t1
     1048                                -- k1 <- repLKind k
     1049                                -- repTSig t1 k1
    10481050repTy (HsSpliceTy splice _)     = repSplice splice
    10491051repTy (HsExplicitListTy _ _ tys) = do
    10501052                                    tys1 <- repLTys tys

It's somewhat unsavory, but it prevents us from having to change a truckload of combinators in Language.Haskell.TH.Lib to take KindQ as an argument instead of Kind.

comment:5 Changed 2 months ago by RyanGlScott

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

Phab:D3751 addresses the DsMeta portion of this ticket.

comment:6 Changed 8 weeks ago by Ryan Scott <ryan.gl.scott@…>

In b3b564fb/ghc:

Merge types and kinds in DsMeta

Summary:
Types and kinds are now the same in GHC... well, except in the code
that involves Template Haskell, where types and kinds are given separate
treatment. This aims to unify that treatment in the `DsMeta` module.

The gist of this patch is replacing all uses of `repLKind` with `repLTy`.
This is isn't quite as simple as one might imagine, since `repLTy` returns a
`Core (Q Type)` (a monadic expression), whereas `repLKind` returns a
`Core Kind` (a pure expression). This causes many awkward impedance mismatches.

One option would be to change every combinator in `Language.Haskell.TH.Lib` to
take `KindQ` as an argument instead of `Kind`. But this would be a breaking
change of colossal proportions.

Instead, this patch takes a somewhat different approach. This migrates the
existing `Language.Haskell.TH.Lib` module to
`Language.Haskell.TH.Lib.Internal`, and changes all `Kind`-related combinators
in `Language.Haskell.TH.Lib.Internal` to live in `Q`. The new
`Language.Haskell.TH.Lib` module then re-exports most of
`Language.Haskell.TH.Lib.Internal` with the exception of the `Kind`-related
combinators, for which it redefines them to be their current definitions (which
don't live in `Q`). This allows us to retain backwards compatibility with
previous `template-haskell` releases, but more importantly, it allows GHC to
make as many changes to the `Internal` code as it wants for its purposes
without fear of disrupting the public API.

This solves half of #11785 (the other half being `TcSplice`).

Test Plan: ./validate

Reviewers: goldfire, austin, bgamari

Reviewed By: goldfire

Subscribers: rwbarton, thomie

GHC Trac Issues: #11785

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

comment:7 Changed 8 weeks ago by RyanGlScott

I looked into completing the TcSplice portion of this ticket recently. While making the change was simple enough, many reified kinds became a lot noisier due to the symptoms described in #14060.

It'd be nice to fix #14060 first so as not to regress the quality of reified kinds.

comment:8 Changed 5 weeks ago by RyanGlScott

Differential Rev(s): Phab:D3751Phab:D3751, Phab:D3854

comment:9 Changed 5 weeks ago by Ryan Scott <ryan.gl.scott@…>

In c948b786/ghc:

Fix #11785 by making reifyKind = reifyType

Summary:
This ties up the last loose end in Template Haskell's separate
code paths for types and kinds. By making `reifyKind = reifyType` in
`TcSplice`, types and kinds are now treated on equal terms in TH.

This is itself a small patch, but most of the heavy lifting to make this
possible was done in ad7b945257ea262e3f6f46daa4ff3e451aeeae0b.

Test Plan: ./validate

Reviewers: goldfire, austin, bgamari

Reviewed By: goldfire

Subscribers: rwbarton, thomie

GHC Trac Issues: #11785

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

comment:10 Changed 5 weeks ago by RyanGlScott

Milestone: 8.4.1
Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.