Split HsConDecl{H98,GADT}Detailswip/T18844-alternate

Haskell98 and GADT constructors both use `HsConDeclDetails`, which includes
`InfixCon`. But `InfixCon` is never used for GADT constructors, which results
in an awkward unrepresentable state. This removes the unrepresentable state by:

* Renaming the existing `HsConDeclDetails` synonym to `HsConDeclH98Details`,
  which emphasizes the fact that it is now only used for Haskell98-style data
  constructors, and
* Creating a new `HsConDeclGADTDetails` data type with `PrefixConGADT` and
  `RecConGADT` constructors that closely resemble `PrefixCon` and `InfixCon`
  in `HsConDeclH98Details`. The key difference is that `HsConDeclGADTDetails`
  lacks any way to represent infix constructors.

The rest of the patch is refactoring to accommodate the new structure of
`HsConDecl{H98,GADT}Details`. Some highlights:

* The `getConArgs` and `hsConDeclArgTys` functions have been removed, as
  there is no way to implement these functions uniformly for all
  `ConDecl`s. For the most part, their previous call sites now
  pattern match on the `ConDecl`s directly and do different things for
  `ConDeclH98`s and `ConDeclGADT`s.

  I did introduce one new function to make the transition easier:
  `getRecConArgs_maybe`, which extracts the arguments from a `RecCon(GADT)`.
  This is still possible since `RecCon(GADT)`s still use the same representation
  in both `HsConDeclH98Details` and `HsConDeclGADTDetails`, and since the
  pattern that `getRecConArgs_maybe` implements is used in several places,
  I thought it worthwhile to factor it out into its own function.
* Previously, the `con_args` fields in `ConDeclH98` and `ConDeclGADT` were
  both of type `HsConDeclDetails`. Now, the former is of type
  `HsConDeclH98Details`, and the latter is of type `HsConDeclGADTDetails`,
  which are distinct types. As a result, I had to rename the `con_args` field
  in `ConDeclGADT` to `con_g_args` to make it typecheck.

  A consequence of all this is that the `con_args` field is now partial, so
  using `con_args` as a top-level field selector is dangerous. (Indeed, Haddock
  was using `con_args` at the top-level, which caused it to crash at runtime
  before I noticed what was wrong!) I decided to add a disclaimer in the 9.2.1
  release notes to advertise this pitfall.

Fixes #18844. Bumps the `haddock` submodule.

1 files changed, 35 insertions, 20 deletions

diff --git a/compiler/GHC/Rename/Module.hs b/compiler/GHC/Rename/Module.hs
index 0a4a3e5bdf..2eef0a6db7 100644
--- a/compiler/GHC/Rename/Module.hs
+++ b/compiler/GHC/Rename/Module.hs
@@ -2180,7 +2180,7 @@ rnConDecl decl@(ConDeclH98 { con_name = name, con_ex_tvs = ex_tvs
         ; bindLHsTyVarBndrs ctxt WarnUnusedForalls
                             Nothing ex_tvs $ \ new_ex_tvs ->
     do  { (new_context, fvs1) <- rnMbContext ctxt mcxt
-        ; (new_args,    fvs2) <- rnConDeclDetails (unLoc new_name) ctxt args
+        ; (new_args,    fvs2) <- rnConDeclH98Details (unLoc new_name) ctxt args
         ; let all_fvs  = fvs1 `plusFV` fvs2
         ; traceRn "rnConDecl (ConDeclH98)" (ppr name <+> vcat
              [ text "ex_tvs:" <+> ppr ex_tvs
@@ -2197,15 +2197,12 @@ rnConDecl decl@(ConDeclGADT { con_names   = names
                             , con_forall  = forall@(L _ explicit_forall)
                             , con_qvars   = explicit_tkvs
                             , con_mb_cxt  = mcxt
-                            , con_args    = args
+                            , con_g_args  = args
                             , con_res_ty  = res_ty
                             , con_doc = mb_doc })
   = do  { mapM_ (addLocM checkConName) names
         ; new_names <- mapM lookupLocatedTopBndrRn names
 
-        ; let theta         = hsConDeclTheta mcxt
-              arg_tys       = hsConDeclArgTys args
-
           -- We must ensure that we extract the free tkvs in left-to-right
           -- order of their appearance in the constructor type.
           -- That order governs the order the implicitly-quantified type
@@ -2213,9 +2210,9 @@ rnConDecl decl@(ConDeclGADT { con_names   = names
           -- See #14808.
         ; implicit_bndrs <- forAllOrNothing explicit_forall
             $ extractHsTvBndrs explicit_tkvs
-            $ extractHsTysRdrTyVars theta
-            $ extractHsScaledTysRdrTyVars arg_tys
-            $ extractHsTysRdrTyVars [res_ty] []
+            $ extractHsTysRdrTyVars (hsConDeclTheta mcxt)
+            $ extractConDeclGADTDetailsTyVars args
+            $ extractHsTyRdrTyVars res_ty
 
         ; let ctxt = ConDeclCtx new_names
 
@@ -2223,7 +2220,7 @@ rnConDecl decl@(ConDeclGADT { con_names   = names
           bindLHsTyVarBndrs ctxt WarnUnusedForalls
                             Nothing explicit_tkvs $ \ explicit_tkvs ->
     do  { (new_cxt, fvs1)    <- rnMbContext ctxt mcxt
-        ; (new_args, fvs2)   <- rnConDeclDetails (unLoc (head new_names)) ctxt args
+        ; (new_args, fvs2)   <- rnConDeclGADTDetails (unLoc (head new_names)) ctxt args
         ; (new_res_ty, fvs3) <- rnLHsType ctxt res_ty
 
          -- Ensure that there are no nested `forall`s or contexts, per
@@ -2238,7 +2235,7 @@ rnConDecl decl@(ConDeclGADT { con_names   = names
             (ppr names $$ ppr implicit_tkvs $$ ppr explicit_tkvs)
         ; return (decl { con_g_ext = implicit_tkvs, con_names = new_names
                        , con_qvars = explicit_tkvs, con_mb_cxt = new_cxt
-                       , con_args = new_args, con_res_ty = new_res_ty
+                       , con_g_args = new_args, con_res_ty = new_res_ty
                        , con_doc = mb_doc
                        , con_forall = forall }, -- Remove when #18311 is fixed
                   all_fvs) } }
@@ -2249,27 +2246,45 @@ rnMbContext _    Nothing    = return (Nothing, emptyFVs)
 rnMbContext doc (Just cxt) = do { (ctx',fvs) <- rnContext doc cxt
                                 ; return (Just ctx',fvs) }
 
-rnConDeclDetails
-   :: Name
+rnConDeclH98Details ::
+      Name
    -> HsDocContext
-   -> HsConDetails (HsScaled GhcPs (LHsType GhcPs)) (Located [LConDeclField GhcPs])
-   -> RnM ((HsConDetails (HsScaled GhcRn (LHsType GhcRn))) (Located [LConDeclField GhcRn]),
-           FreeVars)
-rnConDeclDetails _ doc (PrefixCon tys)
+   -> HsConDeclH98Details GhcPs
+   -> RnM (HsConDeclH98Details GhcRn, FreeVars)
+rnConDeclH98Details _ doc (PrefixCon tys)
   = do { (new_tys, fvs) <- mapFvRn (rnScaledLHsType doc) tys
        ; return (PrefixCon new_tys, fvs) }
-
-rnConDeclDetails _ doc (InfixCon ty1 ty2)
+rnConDeclH98Details _ doc (InfixCon ty1 ty2)
   = do { (new_ty1, fvs1) <- rnScaledLHsType doc ty1
        ; (new_ty2, fvs2) <- rnScaledLHsType doc ty2
        ; return (InfixCon new_ty1 new_ty2, fvs1 `plusFV` fvs2) }
+rnConDeclH98Details con doc (RecCon flds)
+  = do { (new_flds, fvs) <- rnRecConDeclFields con doc flds
+       ; return (RecCon new_flds, fvs) }
 
-rnConDeclDetails con doc (RecCon (L l fields))
+rnConDeclGADTDetails ::
+      Name
+   -> HsDocContext
+   -> HsConDeclGADTDetails GhcPs
+   -> RnM (HsConDeclGADTDetails GhcRn, FreeVars)
+rnConDeclGADTDetails _ doc (PrefixConGADT tys)
+  = do { (new_tys, fvs) <- mapFvRn (rnScaledLHsType doc) tys
+       ; return (PrefixConGADT new_tys, fvs) }
+rnConDeclGADTDetails con doc (RecConGADT flds)
+  = do { (new_flds, fvs) <- rnRecConDeclFields con doc flds
+       ; return (RecConGADT new_flds, fvs) }
+
+rnRecConDeclFields ::
+     Name
+  -> HsDocContext
+  -> Located [LConDeclField GhcPs]
+  -> RnM (Located [LConDeclField GhcRn], FreeVars)
+rnRecConDeclFields con doc (L l fields)
   = do  { fls <- lookupConstructorFields con
         ; (new_fields, fvs) <- rnConDeclFields doc fls fields
                 -- No need to check for duplicate fields
                 -- since that is done by GHC.Rename.Names.extendGlobalRdrEnvRn
-        ; return (RecCon (L l new_fields), fvs) }
+        ; pure (L l new_fields, fvs) }
 
 -------------------------------------------------