Simon Peyton Jones pushed to branch wip/T26543b at Glasgow Haskell Compiler / GHC

Commits:

3 changed files:

Changes:

  • compiler/GHC/Tc/Gen/App.hs
    ... ... @@ -2156,8 +2156,7 @@ foldQLInstVars check_tv ty
    2156 2156
     
    
    2157 2157
     qlUnify ::  TcType -> TcType -> TcM ()
    
    2158 2158
     -- Unify ty1 with ty2:
    
    2159
    ---   * It can unify both instantiation variables (possibly with polytypes),
    
    2160
    ---     and ordinary unification variables (but only with monotypes)
    
    2159
    +--   * It unifies /only/ instantiation variables (see (UQL6)), hopefully with polytypes
    
    2161 2160
     --   * It does not return a coercion (unlike unifyType); it is called
    
    2162 2161
     --     for the sole purpose of unifying instantiation variables, although it
    
    2163 2162
     --     may also (opportunistically) unify regular unification variables.
    
    ... ... @@ -2264,11 +2263,11 @@ In qlUnify, if we find (kappa ~ ty), we are going to update kappa := ty.
    2264 2263
     That is the entire point of qlUnify!   Wrinkles:
    
    2265 2264
     
    
    2266 2265
     (UQL1) Before unifying an instantiation variable in `go_flexi`, we must check
    
    2267
    -  the usual unification conditions, by calling `GHC.Tc.Utils.Unify.simpleUnifyCheck`.
    
    2268
    -  For example that checks for
    
    2266
    +  the usual unification conditions. In particular:
    
    2269 2267
         * An occurs-check
    
    2270
    -    * Level mis-match
    
    2271 2268
         * An attempt to unify a concrete type variable with a non-concrete type.
    
    2269
    +  We don't need a level-check because the level of an instantiation variable
    
    2270
    +  is infinity -- see (UQL3).
    
    2272 2271
     
    
    2273 2272
     (UQL2) What if kappa and ty have different kinds?  We simply call the
    
    2274 2273
       ordinary unifier and use the coercion to connect the two.
    
    ... ... @@ -2350,6 +2349,9 @@ That is the entire point of qlUnify! Wrinkles:
    2350 2349
     
    
    2351 2350
       The simplest thing is to say that `qlUnify` unifies /only/ instantiation variables
    
    2352 2351
       (see calls to `isQLInstTyVar`).  We could be more ambitious, perhaps, in future.
    
    2352
    +  Remember, though, that the /only/ reason for QuickLoook is to unify instantiation variables
    
    2353
    +  with polytypes; anything else is optional, and will be picked up with later "ordinary"
    
    2354
    +  typechecking.  So unifing only instantiation variables is enough.
    
    2353 2355
     
    
    2354 2356
     (UQL7) If `qlUnify` sees a concrete `kaapa`, thus
    
    2355 2357
                 kappa[conc] ~ some-type
    

  • compiler/GHC/Tc/Utils/Unify.hs
    ... ... @@ -3631,7 +3631,6 @@ simpleUnifyCheck :: UnifyCheckCaller -> TcLevel -> TcTyVar -> TcType -> SimpleUn
    3631 3631
     -- * Rejects if lhs_tv occurs in rhs_ty (occurs check)
    
    3632 3632
     -- * Rejects foralls unless
    
    3633 3633
     --      lhs_tv is RuntimeUnk (used by GHCi debugger)
    
    3634
    ---          or is a QL instantiation variable
    
    3635 3634
     -- * Rejects a non-concrete type if lhs_tv is concrete
    
    3636 3635
     -- * Rejects type families unless fam_ok=True
    
    3637 3636
     -- * Does a level-check for type variables, to avoid skolem escape
    
    ... ... @@ -3664,11 +3663,10 @@ simpleUnifyCheck caller given_eq_lvl lhs_tv rhs
    3664 3663
            | isCoVar tcv                                      = occ_check
    
    3665 3664
            | tcTyVarLevel tcv `strictlyDeeperThan` lhs_tv_lvl = True
    
    3666 3665
            | lhs_tv_is_concrete, not (isConcreteTyVar tcv)    = True
    
    3667
    -       | lhs_tv_nm == getName tcv                         = True
    
    3668 3666
            | occ_check                                        = True
    
    3669 3667
            | otherwise                                        = False
    
    3670 3668
            where
    
    3671
    -         occ_check = anyFreeVarsOfType rhs_tcv_is_bad (varType tcv)
    
    3669
    +         occ_check = simpleOccursCheck lhs_tv_nm tcv
    
    3672 3670
     
    
    3673 3671
         rhs_is_bad (TyVarTy tv) = rhs_tcv_is_bad tv
    
    3674 3672
     
    
    ... ... @@ -3693,7 +3691,7 @@ simpleUnifyCheck caller given_eq_lvl lhs_tv rhs
    3693 3691
         rhs_is_bad (CoercionTy co)  = co_is_bad co
    
    3694 3692
         rhs_is_bad (LitTy {})       = False
    
    3695 3693
     
    
    3696
    -    -- Coercions: see (SUC1) in Note [simpleUnifyCheck]
    
    3694
    +    -- Coercions: see Note [Occurs check and coercions]
    
    3697 3695
         co_is_bad co = anyFreeVarsOfCo rhs_tcv_is_bad co
    
    3698 3696
     
    
    3699 3697
     {- Note [simpleUnifyCheck]
    
    ... ... @@ -3703,41 +3701,28 @@ It can return a definite decision (SUC_CannotUnify of SUC_CanUnify), or uncertai
    3703 3701
     (SUC_NotSure).  In the latter case we will later use `checkTyEqRhs` to resolve.
    
    3704 3702
     In particular, `simpleUnifyCheck`:
    
    3705 3703
     
    
    3706
    -* Rejects if lhs_tv occurs in rhs_ty (occurs check)
    
    3704
    +* Rejects if lhs_tv occurs in rhs_ty (see Note [Simple occurs check])
    
    3707 3705
     * Rejects foralls unless
    
    3708 3706
           lhs_tv is RuntimeUnk (used by GHCi debugger)
    
    3709
    -          or is a QL instantiation variable
    
    3710 3707
     * Rejects a non-concrete type if lhs_tv is concrete
    
    3711 3708
     * Rejects type families unless fam_ok=True
    
    3712 3709
     * Does a level-check for type variables, to avoid skolem escape
    
    3713 3710
     
    
    3714 3711
     This function is pretty heavily used, so it's optimised not to allocate.
    
    3715 3712
     
    
    3716
    -(SUC1) `simpleUnifyCheck` used by QuickLook's `qlUnify`, and anything other than
    
    3717
    -  SUC_CanUnify will tell `qlUnify` not to proceed.  That makes QuickLook depend
    
    3718
    -  (regrettably, delicately) on the exact choices made by `simpleUnifyCheck`.
    
    3719
    -
    
    3720
    -  A case in point: in #26543. In the repro case in the Descriptions, `qlUnify` failed
    
    3721
    -  because there was a coercion /hole/ in the RHS; but one that was ultimately Refl.
    
    3722
    -
    
    3723
    -  So in `simpleUnifyCheck` we only look at the /kind/ of a coercion, not the
    
    3724
    -  evidence itself.  I'm a bit worried about building a loop, if the evidence
    
    3725
    -  mentions the LHS unification variable; but I can't see how that can happen, and
    
    3726
    -  I /really/ don't want to be super-conservative for coercion holes (#26543).  So,
    
    3727
    -  for now at least, we look just at the kind of the coercion.
    
    3728
    -
    
    3729
    -  Note that this (unsatisfactorily) differs from the choice in `checkCo`, but
    
    3730
    -  changing that too is an unforced change so I have left the inconsistency.
    
    3713
    +Note [Occurs check and coercions]
    
    3714
    +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    3715
    +What if we have
    
    3716
    +    alpha ~ Int |>  (CoHole ch :: Type ~ blah)
    
    3731 3717
     
    
    3732
    -(SUC2) When we come across a type variable, we use `simple_occurs_check`, as also
    
    3733
    -  does `checkTyEqRhs`.  That `simple_occurs_check` checks for occurrences in the
    
    3734
    -  /kind/ if the type variable, using `anyFreeVarsOfType`.  And that /does/ look
    
    3735
    -  inside coercions; while for coercion holes it just looks in the type of the
    
    3736
    -  coercion hole.
    
    3718
    +Clearly we want to check that `alpha` doesn't appear free in `blah`.  But even if
    
    3719
    +it doesn't, what if `ch` is later filled in with a coercion mentioning `alpha`?
    
    3720
    +Tht would be bad: we'd get an infinite structure.
    
    3737 3721
     
    
    3738
    -  This is all sadly inconsistent with (SUC1), but it's not currently causing
    
    3739
    -  problems, so I am just leaving breadcrumbs (this Note) to point this out for
    
    3740
    -  now.
    
    3722
    +The right thing to do is to check for a loop when (later) filling in `ch`.
    
    3723
    +We don't do that right now, and it's very obscure.  But in `simpleUnifyCheck`
    
    3724
    +we just look in the free vars of the /kind/ of the hole, which is what
    
    3725
    +`anyFreeVarsOfCo` does.
    
    3741 3726
     -}
    
    3742 3727
     
    
    3743 3728
     {- *********************************************************************
    
    ... ... @@ -4371,9 +4356,8 @@ checkCo flags co =
    4371 4356
             , LC_Promote { lc_lvlp = lhs_tv_lvl } <- lc
    
    4372 4357
             -> do { reason <- checkPromoteFreeVars occ_prob lhs_tv lhs_tv_lvl $
    
    4373 4358
                               tyCoVarsOfCo co
    
    4374
    -                          -- Maybe we should just check the free vars of the
    
    4375
    -                          -- /type/ of the coercion, to line up with
    
    4376
    -                          -- (SUC1) in Note [simpleUnifyCheck]
    
    4359
    +                           -- See Note [Occurs check and coercions]
    
    4360
    +                          --  and Note [checkCo] (2)
    
    4377 4361
                   ; return $
    
    4378 4362
                     if cterHasNoProblem reason
    
    4379 4363
                     then pure co
    
    ... ... @@ -4670,7 +4654,7 @@ checkTyVar flags occ_tv
    4670 4654
                       -- so the only reason for a filled meta-tyvar is promotion
    
    4671 4655
     
    
    4672 4656
                     else do_rhs_checks
    
    4673
    -                     [ simpleOccursCheck      occ  occ_tv
    
    4657
    +                     [ tyVarOccursCheck      occ  occ_tv
    
    4674 4658
                          , tyVarLevelCheck        lc   occ_tv
    
    4675 4659
                          , tyVarConcretenessCheck conc occ_tv ]
    
    4676 4660
                   }
    
    ... ... @@ -4728,20 +4712,25 @@ promotionDone occ_tv _ (CC_Promote {}) = isFilledMetaTyVar occ_tv
    4728 4712
     promotionDone _      _               _ = return False
    
    4729 4713
     
    
    4730 4714
     ---------------------
    
    4731
    -simpleOccursCheck :: OccursCheck -> TcTyVar -> TyVarCheckResult m
    
    4715
    +tyVarOccursCheck :: OccursCheck -> TcTyCoVar -> TyVarCheckResult m
    
    4732 4716
     -- ^ The "interpreter" for 'OccursCheck'
    
    4733
    --- Check for an occurrence of lhs_tv in occ_tv or its kind
    
    4734
    -simpleOccursCheck OC_None _
    
    4717
    +-- Check for an occurrence of lhs_tv in occ_tv /or/ its kind
    
    4718
    +tyVarOccursCheck OC_None _
    
    4735 4719
       = TyVarCheck_Success
    
    4736
    -simpleOccursCheck (OC_Check lhs_tv occ_prob) occ_tv
    
    4737
    -  | is_bad occ_tv = TyVarCheck_Error (cteProblem occ_prob)
    
    4738
    -  | otherwise     = TyVarCheck_Success
    
    4720
    +tyVarOccursCheck (OC_Check { occurs_tv_name = lhs_tv, occurs_problem = prob }) occ_tcv
    
    4721
    +  | simpleOccursCheck lhs_tv occ_tcv = TyVarCheck_Error (cteProblem prob)
    
    4722
    +  | otherwise                        = TyVarCheck_Success
    
    4723
    +
    
    4724
    +simpleOccursCheck :: Name -> TyCoVar -> Bool
    
    4725
    +-- Check for an occurrence of lhs_tv in occ_tv /or/ its kind
    
    4726
    +-- True <=> occurs check fires
    
    4727
    +simpleOccursCheck lhs_tv occ_tcv
    
    4728
    +  = is_bad occ_tcv
    
    4739 4729
       where
    
    4740
    -    -- See (SUC2) in Note [simpleUnifyCheck]
    
    4741 4730
         is_bad :: TyCoVar -> Bool
    
    4742
    -    is_bad occ_tv | lhs_tv == tyVarName occ_tv                     = True
    
    4743
    -                  | anyFreeVarsOfType is_bad $! (tyVarKind occ_tv) = True
    
    4744
    -                  | otherwise                                      = False
    
    4731
    +    is_bad occ_tcv | lhs_tv == tyVarName occ_tcv                   = True
    
    4732
    +                   | anyFreeVarsOfType is_bad $! (varType occ_tcv) = True
    
    4733
    +                   | otherwise                                     = False
    
    4745 4734
     
    
    4746 4735
     -------------------------
    
    4747 4736
     tyVarLevelCheck :: LevelCheck m -> TcTyVar -> TyVarCheckResult m
    

  • testsuite/tests/typecheck/should_fail/T26543a.hs
    ... ... @@ -4,7 +4,7 @@
    4 4
     -- but was erroneously accpeted with ImpredicativeTypes
    
    5 5
     -- See !16566
    
    6 6
     
    
    7
    -module T26543 where
    
    7
    +module T26543a where
    
    8 8
     
    
    9 9
     import Data.Kind
    
    10 10
     import Data.Proxy