Add comments about pretty-printing via IfaceSyn

Provoked by discussion on Phab:D5097 (Trac #15546), I'm adding
a big Note explaing the strategy of pretty-printing via IfaceSyn

4 files changed, 67 insertions, 55 deletions

diff --git a/compiler/iface/IfaceSyn.hs b/compiler/iface/IfaceSyn.hs
index 7445ce9c50..9fcf5dcbee 100644
--- a/compiler/iface/IfaceSyn.hs
+++ b/compiler/iface/IfaceSyn.hs
@@ -192,6 +192,7 @@ data IfaceFamTyConFlav
   | IfaceClosedSynFamilyTyCon (Maybe (IfExtName, [IfaceAxBranch]))
     -- ^ Name of associated axiom and branches for pretty printing purposes,
     -- or 'Nothing' for an empty closed family without an axiom
+    -- See Note [Pretty-printing via IfaceSyn] in PprTyThing
   | IfaceAbstractClosedSynFamilyTyCon
   | IfaceBuiltInSynFamTyCon -- for pretty printing purposes only
 
diff --git a/compiler/iface/IfaceType.hs b/compiler/iface/IfaceType.hs
index 2fe3fe02eb..06ea8ff8db 100644
--- a/compiler/iface/IfaceType.hs
+++ b/compiler/iface/IfaceType.hs
@@ -117,7 +117,7 @@ type IfaceKind     = IfaceType
 -- | A kind of universal type, used for types and kinds.
 --
 -- Any time a 'Type' is pretty-printed, it is first converted to an 'IfaceType'
--- before being printed. See @Note [IfaceType and pretty-printing]@.
+-- before being printed. See Note [Pretty printing via IfaceSyn] in PprTyThing
 data IfaceType
   = IfaceFreeTyVar TyVar                -- See Note [Free tyvars in IfaceType]
   | IfaceTyVar     IfLclName            -- Type/coercion variable only, not tycon
@@ -143,28 +143,6 @@ data IfaceType
 type IfacePredType = IfaceType
 type IfaceContext = [IfacePredType]
 
-{-
-Note [IfaceType and pretty-printing]
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-IfaceType has a dual role. Similarly to other Iface data types, it is used as a
-serialization mechanism for Type when writing to and reading from interface
-files. Less obviously, it is also a vehicle for pretty-printing. Any time that
-a Type is pretty-printed, it is first converted to an IfaceType and /then/
-printed out.
-
-Why go through all this trouble? One major reason for this is that an IfaceType
-stores slightly more information about its structure than a Type does, which
-makes certain pretty-printing decisions easier. Most notably, in type
-application forms (such as IfaceAppTy, IfaceTyConApp, and IfaceTupleTy), we
-track whether each of the arguments to a function are visible or not, which
-makes it easier to suppress printing out the invisible arguments.
-See Note [Suppressing invisible arguments] for more.
-
-Another minor benefit of using IfaceTypes for pretty-printing is that this
-avoids the need to duplicate code between the Outputable instances for Type
-and IfaceType.
--}
-
 data IfaceTyLit
   = IfaceNumTyLit Integer
   | IfaceStrTyLit FastString
@@ -229,8 +207,8 @@ data IfaceTyConSort = IfaceNormalTyCon          -- ^ a regular tycon
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Nowadays (since Nov 16, 2016) we pretty-print a Type by converting to
 an IfaceType and pretty printing that.  This eliminates a lot of
-pretty-print duplication, and it matches what we do with
-pretty-printing TyThings.
+pretty-print duplication, and it matches what we do with pretty-
+printing TyThings. See Note [Pretty printing via IfaceSyn] in PprTyThing.
 
 It works fine for closed types, but when printing debug traces (e.g.
 when using -ddump-tc-trace) we print a lot of /open/ types.  These
diff --git a/compiler/main/PprTyThing.hs b/compiler/main/PprTyThing.hs
index 35741b877d..b0a72cf499 100644
--- a/compiler/main/PprTyThing.hs
+++ b/compiler/main/PprTyThing.hs
@@ -37,39 +37,68 @@ import Outputable
 -- -----------------------------------------------------------------------------
 -- 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,
+{- Note [Pretty printing via IfaceSyn]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Our general plan for prett-printing
+  - Types
+  - TyCons
+  - Classes
+  - Pattern synonyms
+  ...etc...
+
+is to convert them to IfaceSyn, and pretty-print that. For example
+  - pprType converts a Type to an IfaceType, and pretty prints that.
+  - pprTyThing converts the TyThing to an IfaceDecl,
+    and pretty prints that.
+
+So IfaceSyn play a dual role:
+  - it's the internal version of an interface files
+  - it's used for pretty-printing
+
+Why do this?
+
+* A significant reason is that we need to be able
+  to pretty-print IfaceSyn (to display Foo.hi), and it was a
+  pain to duplicate masses of pretty-printing goop, esp for
+  Type and IfaceType.
+
+* When pretty-printing (a type, say), we want to tidy (with
+  tidyType) to avoids having (forall a a. blah) where the two
+  a's have different uniques.
+
+  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.
+* Interface files contains fast-strings, not uniques, so the very same
+  tidying must take place when we convert to IfaceDecl. E.g.
+  MkIface.tyThingToIfaceDecl which converts a TyThing (i.e. TyCon,
+  Class etc) to an IfaceDecl.
+
+  Bottom line: IfaceDecls are already 'tidy', so it's straightforward
+  to print them.
+
+* An alternative I once explored was 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.
+
+Consequences:
+
+- IfaceSyn (and IfaceType) must contain enough information to
+  print nicely.  Hence, for example, the IfaceAppArgs type, which
+  allows us to suppress invisible kind arguments in types
+  (see Note [Suppressing invisible arguments] in IfaceType)
+
+- In a few places we have info that is used only for pretty-printing,
+  and is totally ignored when turning IfaceSyn back into TyCons
+  etc (in TcIface). For example, IfaceClosedSynFamilyTyCon
+  stores a [IfaceAxBranch] that is used only for pretty-printing.
+
+- See Note [Free tyvars in IfaceType] in IfaceType
 
 See #7730, #8776 for details   -}
 
diff --git a/compiler/types/TyCoRep.hs b/compiler/types/TyCoRep.hs
index fd3d1dfff7..81cd2b0f95 100644
--- a/compiler/types/TyCoRep.hs
+++ b/compiler/types/TyCoRep.hs
@@ -2713,7 +2713,11 @@ to an @IfaceType@. See Note [IfaceType and pretty-printing] in IfaceType.
 See Note [Precedence in types] in BasicTypes.
 -}
 
-------------------
+--------------------------------------------------------
+-- When pretty-printing types, we convert to IfaceType,
+--   and pretty-print that.
+-- See Note [Pretty printing via IfaceSyn] in PprTyThing
+--------------------------------------------------------
 
 pprType, pprParendType :: Type -> SDoc
 pprType       = pprPrecType topPrec