Re: [GHC] #13959: substTyVar's definition is highly dubious

19 Jul 2017

      #13959: substTyVar's definition is highly dubious
-------------------------------------+-------------------------------------
        Reporter:  RyanGlScott       |                Owner:  (none)
            Type:  bug               |               Status:  patch
        Priority:  normal            |            Milestone:
       Component:  Compiler          |              Version:  8.0.1
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  None/Unknown      |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:                    |  Differential Rev(s):  Phab:D3732
       Wiki Page:                    |
-------------------------------------+-------------------------------------

Comment (by simonpj):

 I differ right at the start!

 If you look at the corresponding code for term variables you'll see (in
 `CoreSubst`):
 {{{
 lookupIdSubst :: SDoc -> Subst -> Id -> CoreExpr
 lookupIdSubst doc (Subst in_scope ids _ _) v
   | not (isLocalId v) = Var v
   | Just e  <- lookupVarEnv ids       v = e
   | Just v' <- lookupInScope in_scope v = Var v'
         -- Vital! See Note [Extending the Subst]
   | otherwise = WARN( True, text "CoreSubst.lookupIdSubst" <+> doc <+> ppr
 v
                             $$ ppr in_scope)
                 Var v
 }}}
 We do ''not'' apply the substition to the type of the `Id` and update its
 type!  Rather, we just look it up in the in-scope set: that should include
 all in-scope `Id`s; and moreover they are all full-substituted `OutIds`.
 A consequence is that all occurrences of the `Id` are common'd up to point
 to a single `Id` data structure, rather than having lots of copies of the
 `Id`.

 We should do the same for types.  In `substTyVar`, if the var isn't in the
 `tenv` look in the in-scope set.  It should be there.

 Of course, because we have not yet made all calls to `substTy` have the
 right in-scope set, we may not find it in the in-scope set.  I suppose we
 can emit a warning, or just silently accept (as of course we are doing
 right now).  Once we've fixed all the calls to `substTy` we should
 definitely make it a warning.

 So it's an easy fix.

 Did you say you'd found other lurking bugs as a result?

-- 
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13959#comment:7
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler