This week

[Simon Peyton-Jones]

Ian,

You've probably forgotten this, but I finally got around to a tidy-up I've been meaning to do for some time

Sat Sep 20 18:52:38 GMT Daylight Time 2008  simonpj at microsoft.com
  * Tidy up the treatment of dead binders

| From: Ian Lynagh [mailto:igloo at earth.li]
| Sent: 10 February 2008 23:05
....
| While I was looking at this I also got very confused by
|     data InScopeSet = InScope (VarEnv Var) FastInt
| in basicTypes/VarEnv.lhs. The Var/Env/ in this is initialised with
| emptyVar/Set/
|     emptyInScopeSet :: InScopeSet
|     emptyInScopeSet = InScope emptyVarSet (_ILIT(1))
| but elsewhere it doesn't look like it is assumed to obey the VarSet
| invariant:
|     lookupInScope :: InScopeSet -> Var -> Maybe Var
|     lookupInScope (InScope in_scope n) v
|       = go v
|       where
|         go v = case lookupVarEnv in_scope v of
|             Just v' | v == v'   -> Just v'  -- Reached a fixed point
|                 | otherwise -> go v'
|             Nothing         -> Nothing
| In still other places, e.g. coreEqType in types/Type.lhs, the set
| operators are used again. I assume this is all deliberate, but it feels
| very scary to me!

You're right that it's a bit odd. I've tried to document it better at the definition of VarEnv.InScopeSet.

Furthermore, I removed the most troubling aspect altogether, where the "set" could map one Id to a *different* one:
  * Another benefit of the binder-swap change is that I could get rid of
    the Simplifier hack (working, but hacky) in which the InScopeSet was
    used to map a variable to a *different* variable. That allowed me
    to remove VarEnv.modifyInScopeSet, and to simplify lookupInScopeSet
    so that it doesn't look for a fixpoint.  This fixes no bugs, but
    is a useful cleanup.

Simon