Make sure ModIface values are still forced even if not writtenwip/stricter-modiface

When we are not writing a ModIface to disk then the result can retain a
lot of stuff. For example, in the case I was debugging the DocDeclsMap
field was holding onto the entire HomePackageTable due to a single
unforced thunk. Therefore, now if we're not going to write the interface
then we still force deeply it in order to remove these thunks.

The fields in the data structure are not made strict because when we
read the field from the interface we don't want to load it immediately
as there are parts of an interface which are unused a lot of the time.

Also added a note to explain why not all the fields in a ModIface field
are strict.

The result of this is being able to load Agda in ghci and not leaking
information across subsequent reloads.

2 files changed, 45 insertions, 4 deletions

diff --git a/compiler/GHC/Driver/Main.hs b/compiler/GHC/Driver/Main.hs
index 3605b4ac5a..565c7a83c7 100644
--- a/compiler/GHC/Driver/Main.hs
+++ b/compiler/GHC/Driver/Main.hs
@@ -982,7 +982,7 @@ hscMaybeWriteIface logger dflags is_simple iface old_iface mod_location = do
               (const ())
               (writeIface logger profile iface_name iface)
 
-    when (write_interface || force_write_interface) $ do
+    if (write_interface || force_write_interface) then do
 
       -- FIXME: with -dynamic-too, "no_change" is only meaningful for the
       -- non-dynamic interface, not for the dynamic one. We should have another
@@ -1039,6 +1039,9 @@ hscMaybeWriteIface logger dflags is_simple iface old_iface mod_location = do
           let hie_file = ml_hie_file mod_location
           whenM (doesFileExist hie_file) $
             GHC.SysTools.touch logger dflags "Touching hie file" hie_file
+    else
+        -- See Note [Strictness in ModIface]
+        forceModIface iface
 
 --------------------------------------------------------------
 -- NoRecomp handlers
diff --git a/compiler/GHC/Unit/Module/ModIface.hs b/compiler/GHC/Unit/Module/ModIface.hs
index a339df92cc..db7c4ce362 100644
--- a/compiler/GHC/Unit/Module/ModIface.hs
+++ b/compiler/GHC/Unit/Module/ModIface.hs
@@ -24,6 +24,7 @@ module GHC.Unit.Module.ModIface
    , emptyFullModIface
    , mkIfaceHashCache
    , emptyIfaceHashCache
+   , forceModIface
    )
 where
 
@@ -55,6 +56,7 @@ import GHC.Utils.Fingerprint
 import GHC.Utils.Binary
 
 import Control.DeepSeq
+import Control.Exception
 
 {- Note [Interface file stages]
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -145,6 +147,9 @@ type family IfaceBackendExts (phase :: ModIfacePhase) where
 -- except that we explicitly make the 'mi_decls' and a few other fields empty;
 -- as when reading we consolidate the declarations etc. into a number of indexed
 -- maps and environments in the 'ExternalPackageState'.
+--
+-- See Note [Strictness in ModIface] to learn about why some fields are
+-- strict and others are not.
 data ModIface_ (phase :: ModIfacePhase)
   = ModIface {
         mi_module     :: !Module,             -- ^ Name of the module we are for
@@ -228,7 +233,7 @@ data ModIface_ (phase :: ModIfacePhase)
                 -- itself) but imports some trustworthy modules from its own
                 -- package (which does require its own package be trusted).
                 -- See Note [Trust Own Package] in GHC.Rename.Names
-        mi_complete_matches :: [IfaceCompleteMatch],
+        mi_complete_matches :: ![IfaceCompleteMatch],
 
         mi_doc_hdr :: Maybe HsDocString,
                 -- ^ Module header.
@@ -243,7 +248,7 @@ data ModIface_ (phase :: ModIfacePhase)
                 -- ^ Either `()` or `ModIfaceBackend` for
                 -- a fully instantiated interface.
 
-        mi_ext_fields :: ExtensibleFields,
+        mi_ext_fields :: !ExtensibleFields,
                 -- ^ Additional optional fields, where the Map key represents
                 -- the field name, resulting in a (size, serialized data) pair.
                 -- Because the data is intended to be serialized through the
@@ -256,6 +261,29 @@ data ModIface_ (phase :: ModIfacePhase)
                 -- ^ Hash of the .hs source, used for recompilation checking.
      }
 
+{-
+Note [Strictness in ModIface]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The ModIface is the Haskell representation of an interface (.hi) file.
+
+* During compilation we write out ModIface values to disk for files
+  that we have just compiled
+* For packages that we depend on we load the ModIface from disk.
+
+Some fields in the ModIface are deliberately lazy because when we read
+an interface file we don't always need all the parts. For example, an
+interface file contains information about documentation which is often
+not needed during compilation. This is achieved using the lazyPut/lazyGet pair.
+If the field was strict then we would pointlessly load this information into memory.
+
+On the other hand, if we create a ModIface but **don't** write it to
+disk then to avoid space leaks we need to make sure to deepseq all these lazy fields
+because the ModIface might live for a long time (for instance in a GHCi session).
+That's why in GHC.Driver.Main.hscMaybeWriteIface there is the call to
+forceModIface.
+-}
+
 -- | Old-style accessor for whether or not the ModIface came from an hs-boot
 -- file.
 mi_boot :: ModIface -> IsBootInterface
@@ -307,7 +335,7 @@ renameFreeHoles fhs insts =
         -- It wasn't actually a hole
         | otherwise                           = emptyUniqDSet
 
-
+-- See Note [Strictness in ModIface] about where we use lazyPut vs put
 instance Binary ModIface where
    put_ bh (ModIface {
                  mi_module    = mod,
@@ -532,6 +560,16 @@ instance (NFData (IfaceBackendExts (phase :: ModIfacePhase)), NFData (IfaceDeclE
     rnf f16 `seq` f17 `seq` rnf f18 `seq` rnf f19 `seq` f20 `seq` f21 `seq` f22 `seq` rnf f23
     `seq` rnf f24 `seq` f25 `seq` ()
 
+instance NFData (ModIfaceBackend) where
+  rnf (ModIfaceBackend f1 f2 f3 f4 f5 f6 f7 f8 f9 f10 f11 f12 f13)
+    = rnf f1 `seq` rnf f2 `seq` rnf f3 `seq` rnf f4 `seq`
+      rnf f5 `seq` rnf f6 `seq` rnf f7 `seq` rnf f8 `seq`
+      rnf f9 `seq` rnf f10 `seq` rnf f11 `seq` rnf f12 `seq` rnf f13
+
+
+forceModIface :: ModIface -> IO ()
+forceModIface iface = () <$ (evaluate $ force iface)
+
 -- | Records whether a module has orphans. An \"orphan\" is one of:
 --
 -- * An instance declaration in a module other than the definition