Comments only (related to Trac #7730)

2 files changed, 42 insertions, 4 deletions

diff --git a/compiler/iface/IfaceSyn.lhs b/compiler/iface/IfaceSyn.lhs
index 6af5bbefba..79e235925e 100644
--- a/compiler/iface/IfaceSyn.lhs
+++ b/compiler/iface/IfaceSyn.lhs
@@ -1138,6 +1138,8 @@ isIfaceDataInstance IfNoParent = False
 isIfaceDataInstance _          = True
 
 pprIfaceDecl :: ShowSub -> IfaceDecl -> SDoc
+-- NB: pprIfaceDecl is also used for pretty-printing TyThings in GHCi
+--     See Note [Pretty-printing TyThings] in PprTyThing
 pprIfaceDecl ss (IfaceData { ifName = tycon, ifCType = ctype,
                              ifCtxt = context, ifTyVars = tyvars,
                              ifRoles = roles, ifCons = condecls,
diff --git a/compiler/main/PprTyThing.hs b/compiler/main/PprTyThing.hs
index 890502c4f6..d88b137a68 100644
--- a/compiler/main/PprTyThing.hs
+++ b/compiler/main/PprTyThing.hs
@@ -42,6 +42,42 @@ import FastString
 -- -----------------------------------------------------------------------------
 -- Pretty-printing entities that we get from the GHC API
 
+{-  Note [Pretty-printing TyThings]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+We pretty-print a TyThing by converting it to an IfaceDecl,
+and pretty-printing that (see ppr_ty_thing below).
+Here is why:
+
+* When pretty-printing (a type, say), the idiomatic solution is not to
+  "rename type variables on the fly", but rather to "tidy" the type
+  (which gives each variable a distinct print-name), and then
+  pretty-print it (without renaming). Separate the two
+  concerns. Functions like tidyType do this.
+
+* Alas, for type constructors, TyCon, tidying does not work well,
+  because a TyCon includes DataCons which include Types, which mention
+  TyCons. And tidying can't tidy a mutually recursive data structure
+  graph, only trees.
+
+* One alternative would be to ensure that TyCons get type variables
+  with distinct print-names. That's ok for type variables but less
+  easy for kind variables. Processing data type declarations is
+  already so complicated that I don't think it's sensible to add the
+  extra requirement that it generates only "pretty" types and kinds.
+
+*  One place the non-pretty names can show up is in GHCi. But another
+   is in interface files. Look at MkIface.tyThingToIfaceDecl which
+   converts a TyThing (i.e. TyCon, Class etc) to an IfaceDecl. And it
+   already does tidying as part of that conversion!  Why? Because
+   interface files contains fast-strings, not uniques, so the names
+   must at least be distinct.
+
+So if we convert to IfaceDecl, we get a nice tidy IfaceDecl, and can
+print that.  Of course, that means that pretty-printing IfaceDecls
+must be careful to display nice user-friendly results, but that's ok.
+
+See #7730, #8776 for details   -}
+
 --------------------
 -- | Pretty-prints a 'FamInst' (type/data family instance) with its defining location.
 pprFamInst :: FamInst -> SDoc
@@ -98,14 +134,14 @@ pprTyThingInContextLoc tyThing
 
 ------------------------
 ppr_ty_thing :: Bool -> [OccName] -> TyThing -> SDoc
--- NOTE: We pretty-print 'TyThing' via 'IfaceDecl' so that we can reuse the
--- 'TyCon' tidying happening in 'tyThingToIfaceDecl'. See #8776 for details.
+-- We pretty-print 'TyThing' via 'IfaceDecl'
+-- See Note [Pretty-pringint TyThings]
 ppr_ty_thing hdr_only path ty_thing
-  = pprIfaceDecl (ShowSub { ss_how_much = how_much, ss_ppr_bndr = ppr_bndr }) if_decl
+  = pprIfaceDecl ss (tyThingToIfaceDecl ty_thing)
   where
+    ss = ShowSub { ss_how_much = how_much, ss_ppr_bndr = ppr_bndr }
     how_much | hdr_only  = ShowHeader
              | otherwise = ShowSome path
-    if_decl = tyThingToIfaceDecl ty_thing
     name    = getName ty_thing
     ppr_bndr :: OccName -> SDoc
     ppr_bndr | isBuiltInSyntax name