Tidy up and comment wildcards in family instances

I found it was possible to do this a bit more nicely
See Note [Family instance declaration binders] in HsDecls, and
    Note [Wildcards in family instances] in RnSource

compiler/hsSyn/HsDecls.hs

@@ -1189,6 +1189,35 @@ type HsTyPats name = HsImplicitBndrs name [LHsType name]
             -- ^ Type patterns (with kind and type bndrs)
             -- See Note [Family instance declaration binders]
 
+{- Note [Family instance declaration binders]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The HsTyPats field is LHS patterns or a type/data family instance.
+
+The hsib_vars of the HsImplicitBndrs are the template variables of the
+type patterns, i.e. fv(pat_tys).  Note in particular
+
+* The hsib_vars *includes* any anonymous wildcards.  For example
+     type instance F a _ = a
+  The hsib_vars will be {a, _}.  Remember that each separate wildcard
+  '_' gets its own unique.  In this context wildcards behave just like
+  an ordinary type variable, only anonymous.
+
+* The hsib_vars *including* type variables that are already in scope
+
+   Eg   class C s t where
+          type F t p :: *
+        instance C w (a,b) where
+          type F (a,b) x = x->a
+   The hsib_vars of the F decl are {a,b,x}, even though the F decl
+   is nested inside the 'instance' decl.
+
+   However after the renamer, the uniques will match up:
+        instance C w7 (a8,b9) where
+          type F (a8,b9) x10 = x10->a8
+   so that we can compare the type pattern in the 'instance' decl and
+   in the associated 'type' decl
+-}
+
 type TyFamInstEqn  name = TyFamEqn name (HsTyPats name)
 type TyFamDefltEqn name = TyFamEqn name (LHsQTyVars name)
   -- See Note [Type family instance declarations in HsSyn]
@@ -1282,27 +1311,6 @@ data InstDecl name  -- Both class and family instances
   deriving (Typeable)
 deriving instance (DataId id) => Data (InstDecl id)
 
-{-
-Note [Family instance declaration binders]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-A {Ty|Data}FamInstDecl is a data/type family instance declaration
-the pats field is LHS patterns, and the tvs of the HsBSig
-tvs are fv(pat_tys), *including* ones that are already in scope
-
-   Eg   class C s t where
-          type F t p :: *
-        instance C w (a,b) where
-          type F (a,b) x = x->a
-   The tcdTyVars of the F decl are {a,b,x}, even though the F decl
-   is nested inside the 'instance' decl.
-
-   However after the renamer, the uniques will match up:
-        instance C w7 (a8,b9) where
-          type F (a8,b9) x10 = x10->a8
-   so that we can compare the type patter in the 'instance' decl and
-   in the associated 'type' decl
--}
-
 instance (OutputableBndr name) => Outputable (TyFamInstDecl name) where
   ppr = pprTyFamInstDecl TopLevel
 

compiler/rename/RnSource.hs

@@ -46,12 +46,12 @@ import FastString
 import SrcLoc
 import DynFlags
 import HscTypes         ( HscEnv, hsc_dflags )
-import ListSetOps       ( findDupsEq, removeDups )
+import ListSetOps       ( findDupsEq, removeDups, equivClasses )
 import Digraph          ( SCC, flattenSCC, stronglyConnCompFromEdgedVertices )
 import qualified GHC.LanguageExtensions as LangExt
 
 import Control.Monad
-import Data.List ( (\\), nubBy, sortBy )
+import Data.List ( sortBy )
 import Maybes( orElse, mapMaybe )
 import qualified Data.Set as Set ( difference, fromList, toList, null )
 #if __GLASGOW_HASKELL__ < 709
@@ -668,16 +668,14 @@ rnFamInstDecl doc mb_cls tycon (HsIB { hsib_body = pats }) payload rnPayload
                      []             -> pprPanic "rnFamInstDecl" (ppr tycon)
                      (L loc _ : []) -> loc
                      (L loc _ : ps) -> combineSrcSpans loc (getLoc (last ps))
-             -- Duplicates are needed to warn about unused type variables
-             -- See Note [Wild cards in family instances] in TcTyClsDecls
-       ; tv_rdr_names_all <- extractHsTysRdrTyVarsDups pats
-       ; let tv_rdr_names = rmDupsInRdrTyVars tv_rdr_names_all
-             tv_rdr_dups = nubBy eqLocated
-                (freeKiTyVarsTypeVars tv_rdr_names_all
-                 \\ freeKiTyVarsTypeVars tv_rdr_names)
 
+       ; pat_kity_vars_with_dups <- extractHsTysRdrTyVarsDups pats
+             -- Use the "...Dups" form becuase it's needed
+             -- below to report unsed binder on the LHS
        ; var_names <- mapM (newTyVarNameRn mb_cls . L loc . unLoc) $
-                      freeKiTyVarsAllVars tv_rdr_names
+                      freeKiTyVarsAllVars $
+                      rmDupsInRdrTyVars pat_kity_vars_with_dups
+
              -- All the free vars of the family patterns
              -- with a sensible binding location
        ; ((pats', payload'), fvs)
@@ -685,7 +683,16 @@ rnFamInstDecl doc mb_cls tycon (HsIB { hsib_body = pats }) payload rnPayload
                  do { (pats', pat_fvs) <- rnLHsTypes (FamPatCtx tycon) pats
                     ; (payload', rhs_fvs) <- rnPayload doc payload
 
-                    ; tv_nms_dups <- mapM (lookupOccRn . unLoc) tv_rdr_dups
+                       -- Report unused binders on the LHS
+                       -- See Note [Unused type variables in family instances]
+                    ; let groups :: [[Located RdrName]]
+                          groups = equivClasses cmpLocated $
+                                   freeKiTyVarsAllVars pat_kity_vars_with_dups
+                    ; tv_nms_dups <- mapM (lookupOccRn . unLoc) $
+                                     [ tv | (tv:_:_) <- groups ]
+                          -- Add to the used variables any variables that
+                          -- appear *more than once* on the LHS
+                          -- e.g.   F a Int a = Bool
                     ; let tv_nms_used = extendNameSetList rhs_fvs tv_nms_dups
                     ; warnUnusedMatches var_names tv_nms_used
 
@@ -701,9 +708,17 @@ rnFamInstDecl doc mb_cls tycon (HsIB { hsib_body = pats }) payload rnPayload
                     ; unless (null bad_tvs) (badAssocRhs bad_tvs)
                     ; return ((pats', payload'), rhs_fvs `plusFV` pat_fvs) }
 
-       ; let all_fvs = fvs `addOneFV` unLoc tycon'
+       ; let anon_wcs = concatMap collectAnonWildCards pats'
+             all_ibs  = anon_wcs ++ var_names
+                        -- all_ibs: include anonymous wildcards in the implicit
+                        -- binders In a type pattern they behave just like any
+                        -- other type variable except for being anoymous.  See
+                        -- Note [Wildcards in family instances]
+             all_fvs  = fvs `addOneFV` unLoc tycon'
+
        ; return (tycon',
-                 HsIB { hsib_body = pats', hsib_vars = var_names },
+                 HsIB { hsib_body = pats'
+                      , hsib_vars = all_ibs },
                  payload',
                  all_fvs) }
              -- type instance => use, hence addOneFV
@@ -781,7 +796,46 @@ rnATInstDecls rnFun cls tv_ns at_insts
   = rnList (rnFun (Just (cls, tv_ns))) at_insts
     -- See Note [Renaming associated types]
 
-{-
+{- Note [Wildcards in family instances]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Wild cards can be used in type/data family instance declarations to indicate
+that the name of a type variable doesn't matter. Each wild card will be
+replaced with a new unique type variable. For instance:
+
+    type family F a b :: *
+    type instance F Int _ = Int
+
+is the same as
+
+    type family F a b :: *
+    type instance F Int b = Int
+
+This is implemented as follows: during renaming anonymous wild cards
+'_' are given freshly generated names. These names are collected after
+renaming (rnFamInstDecl) and used to make new type variables during
+type checking (tc_fam_ty_pats). One should not confuse these wild
+cards with the ones from partial type signatures. The latter generate
+fresh meta-variables whereas the former generate fresh skolems.
+
+Note [Unused type variables in family instances]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+When the flag -fwarn-unused-matches is on, the compiler reports warnings
+about unused type variables. (rnFamInstDecl) A type variable is considered
+used
+ * when it is either occurs on the RHS of the family instance, or
+   e.g.   type instance F a b = a    -- a is used on the RHS
+
+ * it occurs multiple times in the patterns on the LHS
+   e.g.   type instance F a a = Int  -- a appears more than once on LHS
+
+As usual, the warnings are not reported for for type variables with names
+beginning with an underscore.
+
+Extra-constraints wild cards are not supported in type/data family
+instance declarations.
+
+Relevant tickets: #3699, #10586 and #10982.
+
 Note [Renaming associated types]
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Check that the RHS of the decl mentions only type variables

compiler/typecheck/TcTyClsDecls.hs

@@ -33,7 +33,6 @@ import TcClassDcl
 import TcUnify
 import TcHsType
 import TcMType
-import RnTypes( collectAnonWildCards )
 import TcType
 import FamInst
 import FamInstEnv
@@ -46,7 +45,6 @@ import CoAxiom
 import TyCon
 import DataCon
 import Id
--- import IdInfo
 import Var
 import VarEnv
 import VarSet
@@ -1122,15 +1120,11 @@ tc_fam_ty_pats :: FamTyConShape
 -- (and, if C is poly-kinded, so will its kind parameter).
 
 tc_fam_ty_pats (name, _, kind) mb_clsinfo
-               (HsIB { hsib_body = arg_pats, hsib_vars = vars })
+               (HsIB { hsib_body = arg_pats, hsib_vars = tv_names })
                kind_checker
-  = do { -- See Note [Wild cards in family instances]
-       ; let wcs      = concatMap collectAnonWildCards arg_pats
-             tv_names = vars ++ wcs
-
-         -- Kind-check and quantify
+  = do { -- Kind-check and quantify
          -- See Note [Quantifying over family patterns]
-       ; (_, (res_kind, typats)) <- tcImplicitTKBndrs tv_names $
+         (_, (res_kind, typats)) <- tcImplicitTKBndrs tv_names $
          do { (res_kind, args, leftovers, n)
                 <- tcInferArgs name kind (snd <$> mb_clsinfo) arg_pats
             ; case leftovers of
@@ -1290,42 +1284,6 @@ none. The role of the kind signature (a :: Maybe k) is to add a constraint
 that 'a' must have that kind, and to bring 'k' into scope.
 
 
-Note [Wild cards in family instances]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Wild cards can be used in type/data family instance declarations to indicate
-that the name of a type variable doesn't matter. Each wild card will be
-replaced with a new unique type variable. For instance:
-
-    type family F a b :: *
-    type instance F Int _ = Int
-
-is the same as
-
-    type family F a b :: *
-    type instance F Int b = Int
-
-This is implemented as follows: during renaming anonymous wild cards are given
-freshly generated names. These names are collected after renaming
-(rnFamInstDecl) and used to make new type variables during type checking
-(tc_fam_ty_pats). One should not confuse these wild cards with the ones from
-partial type signatures. The latter generate fresh meta-variables whereas the
-former generate fresh skolems.
-
-When the flag -fwarn-unused-matches is on, the compiler reports warnings
-about unused type variables. (rnFamInstDecl) A type variable is considered
-used when it is either occurs on the RHS of the family instance, or it occurs
-multiple times in the patterns on the LHS. In the first case, the variable
-is in the set of free variables returned by rnPayload. In the second case, there
-are multiple occurences of it in FreeKiTyVars returned by the rmDupsInRdrTyVars.
-
-The warnings are not reported for anonymous wild cards and for type variables
-with names beginning with an underscore.
-
-Extra-constraints wild cards are not supported in type/data family
-instance declarations.
-
-Relevant tickets: #3699, #10586 and #10982.
 
 ************************************************************************
 *                                                                      *