[GHC] #13311: Audit shady uses of tcSplitSigmaTy

21 Feb 2017

      #13311: Audit shady uses of tcSplitSigmaTy
-------------------------------------+-------------------------------------
           Reporter:  RyanGlScott    |             Owner:  (none)
               Type:  task           |            Status:  new
           Priority:  normal         |         Milestone:  8.4.1
          Component:  Compiler       |           Version:  8.1
  (Type checker)                     |
           Keywords:                 |  Operating System:  Unknown/Multiple
       Architecture:                 |   Type of failure:  None/Unknown
  Unknown/Multiple                   |
          Test Case:                 |        Blocked By:
           Blocking:                 |   Related Tickets:
Differential Rev(s):                 |         Wiki Page:
-------------------------------------+-------------------------------------
 When fixing #12918, I ended up needing to introduce a helper function
 called `tcSplitNestedSigmaTys`, which is quite similar to
 `tcSplitSigmaTy`, except that it attempts to look through as many nested
 `forall`s as possible. (See the documentation for `tcSplitNestedSigmaTys`
 [http://git.haskell.org/ghc.git/blob/611f998fd545b45167170d9e60b7d9147178f0a1...
 here]).

 In the process, I accidentally discovered a bug! First, consider this
 program:

 {{{#!hs
 {-# LANGUAGE RankNTypes #-}
 module Bug where

 class C a where
   op :: forall b c. (Monoid b, Monoid c, Eq a) => a -> b -> c
 }}}

 This should be (and is) rejected, since we're trying to constrain `a` with
 `Eq` in `op`, but we don't have `-XConstraintedClassMethods` enabled. But
 what about this equivalent program?

 {{{#!hs
 {-# LANGUAGE RankNTypes #-}
 module Bug where

 class C a where
   op :: forall b. Monoid b => forall c. Monoid c => Eq a => a -> b -> c
 }}}

 By the same logic, this should also be rejected. But in GHC 8.0.2, it
 wasn't! That's because we were checking for the presence of constrained
 class variables using `tcSplitSigmaTy`, which only gives you `Monoid b` as
 the constraint, totally ignoring the bogus constraint inside of `forall c.
 Monoid c => Eq a => a -> b -> c`.

 When I fixed #12918 (a seemingly unrelated task), I ended up replacing
 that very use of `tcSplitSigmaTy` with `tcSplitNestedSigmaTys`. Now it
 gives you `(Monoid b, Monoid c, Eq a)` as expected when checking the
 constraints, and the second program is now rightly rejected on GHC HEAD.

 But I fear that there may be many more bogus uses of `tcSplitSigmaTy`. I
 should go through and try flushing out bugs of a similar caliber, and see
 if subbing in `tcSplitNestedSigmaTys` fixes these bugs.

--
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/13311
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler