simonpj at microsoft.com
Sat Sep 20 14:08:42 EDT 2008
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
| emptyInScopeSet :: InScopeSet
| emptyInScopeSet = InScope emptyVarSet (_ILIT(1))
| but elsewhere it doesn't look like it is assumed to obey the VarSet
| lookupInScope :: InScopeSet -> Var -> Maybe Var
| lookupInScope (InScope in_scope n) v
| = go v
| 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.
More information about the Cvs-ghc