[GHC] #15345: Duplication between `CoreSubst` and `SimplEnv` is very unfortunate

[GHC]

#15345: Duplication between `CoreSubst` and `SimplEnv` is very unfortunate
-------------------------------------+-------------------------------------
        Reporter:  mpickering        |                Owner:  (none)
            Type:  task              |               Status:  new
        Priority:  normal            |            Milestone:  8.8.1
       Component:  Compiler          |              Version:  8.4.3
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 > Help me Simon!

 I agree this is vary far from ideal

 * `CoreSubst.substIdBndr` takes a `Subst`
    * It substitutes in the `IdInfo`

 * `CoreOpt.subst_opt_id_bndr` is similar to `CoreSubst.substIdBndr`, but
   * It zaps the `IdInfo`.

 * `SimplEnv.substIdBndr` takes a `SimplEnv`.
    * For `CoVars` it makes a `TCvSubst`, and calls
 `TyCoRep.substCoVarBndr`
    * For `Ids` it calls `SimplEnv.substNonCoVarBndr`, which
        * zaps the `IdInfo` (because the simplifier will simplify and re-
 add it)
        * and for `Ids` it can cope with join points; I'll add a Note to
 explain this.

 * Why are `CoVars` treated differently to other `Ids`?  Because they can
 occur in types, so their bindings must be in the `TCvSubst`.

 Things we could improve relatively easily:

 * `SimplEnv.substIdBndr` does the Id/CoVar split, calling `substCoVarBndr`
 and `substNonCoVarIdBndr` resp.  But `CoreSubst.substIdBndr` assumes a
 non-CoVar Id; the split is done by `CoreSubst.substBndr`.  This is
 inconsistent.

 * `CoreSubst.substIdBndr` should call `CoreSubst.substIdType` rather than
 copying its code.

 * `SimplEnv.substIdType` could probably just call `CoreSubst.substIdType`

 * I suppose that `CoreOpt.subst_opt_id_bndr` could zap the `IdInfo` and
 then call `CoreSubst.substIdBndr`.  Slightly less efficient but more
 modular; and this is not heavily used code.

 Does any of that help?

-- 
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/15345#comment:3>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler