Improve the documentation on the regression test

Author: ndmitchell

tests/Regressions.hs

@@ -79,28 +79,37 @@ mapFromKeys keys = HM.fromList (zip keys (repeat ()))
 ------------------------------------------------------------------------
 -- Issue #254
 
-data KC = KC Int
+-- Key type that always collides.
+newtype KC = KC Int
   deriving (Eq, Ord, Show)
 instance Hashable KC where
   hashWithSalt salt _ = salt
 
 issue254Lazy :: Assertion
 issue254Lazy = issue254LazyLambda 2
 
--- Important that oldV is not hoisted out by optimisation, so use NOINLINE
+-- We want to make sure that old values in the HashMap are evicted when new values are inserted,
+-- even if they aren't evaluated. To do that, we use the WeakPtr trick described at
+-- http://simonmar.github.io/posts/2018-06-20-Finding-fixing-space-leaks.html.
+-- We insert a value named oldV into the HashMap, then insert over it, checking oldV is no longer reachable.
+--
+-- To make the test robust, it's important that oldV isn't hoisted up to the top or shared. To do that,
+-- we use NOINLINE, make oldV dependent on an unseen argument, and insert _ <- return () to ensure oldV
+-- is under a lambda.
 {-# NOINLINE issue254LazyLambda #-}
 issue254LazyLambda :: Int -> Assertion
 issue254LazyLambda i = do
   _ <- return ()
-  let oldV = show i
+  let oldV = error $ "Should not be evaluated: " ++ show i
   weakV <- mkWeakPtr oldV Nothing
-  let mp = HML.insert (KC 1) "3" $ HML.fromList [(KC 0, "1"), (KC 1, oldV)]
+  let mp = HML.insert (KC 1) (error "Should not be evaluated") $ HML.fromList [(KC 0, "1"), (KC 1, oldV)]
   _ <- evaluate mp
   performGC
   res <- deRefWeak weakV
   _ <- evaluate mp
   assert $ isNothing res
 
+-- Like issue254Lazy, but using strict HashMap
 issue254Strict :: Assertion
 issue254Strict = issue254StrictLambda 2