From 7ab6ab093c86227b6d33a5185ebbd11928ac9754 Mon Sep 17 00:00:00 2001 From: Richard Eisenberg Date: Wed, 29 Apr 2020 17:14:53 +0100 Subject: Refactor hole constraints. Previously, holes (both expression holes / out of scope variables and partial-type-signature wildcards) were emitted as *constraints* via the CHoleCan constructor. While this worked fine for error reporting, there was a fair amount of faff in keeping these constraints in line. In particular, and unlike other constraints, we could never change a CHoleCan to become CNonCanonical. In addition: * the "predicate" of a CHoleCan constraint was really the type of the hole, which is not a predicate at all * type-level holes (partial type signature wildcards) carried evidence, which was never used * tcNormalise (used in the pattern-match checker) had to create a hole constraint just to extract it again; it was quite messy The new approach is to record holes directly in WantedConstraints. It flows much more nicely now. Along the way, I did some cleaning up of commentary in GHC.Tc.Errors.Hole, which I had a hard time understanding. This was instigated by a future patch that will refactor the way predicates are handled. The fact that CHoleCan's "predicate" wasn't really a predicate is incompatible with that future patch. No test case, because this is meant to be purely internal. It turns out that this change improves the performance of the pattern-match checker, likely because fewer constraints are sloshing about in tcNormalise. I have not investigated deeply, but an improvement is not a surprise here: ------------------------- Metric Decrease: PmSeriesG ------------------------- --- compiler/GHC/Tc/Gen/Expr.hs | 13 +++++-------- compiler/GHC/Tc/Gen/HsType.hs | 12 ++++++------ compiler/GHC/Tc/Gen/Rule.hs | 6 ++---- 3 files changed, 13 insertions(+), 18 deletions(-) (limited to 'compiler/GHC/Tc/Gen') diff --git a/compiler/GHC/Tc/Gen/Expr.hs b/compiler/GHC/Tc/Gen/Expr.hs index fbc6c5ba58..3a89daac0b 100644 --- a/compiler/GHC/Tc/Gen/Expr.hs +++ b/compiler/GHC/Tc/Gen/Expr.hs @@ -36,7 +36,6 @@ import {-# SOURCE #-} GHC.Tc.Gen.Splice( tcSpliceExpr, tcTypedBracket, tcUntyp import GHC.Builtin.Names.TH( liftStringName, liftName ) import GHC.Hs -import GHC.Tc.Types.Constraint ( HoleSort(..) ) import GHC.Tc.Utils.Zonk import GHC.Tc.Utils.Monad import GHC.Tc.Utils.Unify @@ -1405,22 +1404,21 @@ Suppose 'wurble' is not in scope, and we have Then the renamer will make (HsUnboundVar "wurble) for 'wurble', and the typechecker will typecheck it with tcUnboundId, giving it -a type 'alpha', and emitting a deferred CHoleCan constraint, to -be reported later. +a type 'alpha', and emitting a deferred Hole, to be reported later. But then comes the visible type application. If we do nothing, we'll generate an immediate failure (in tc_app_err), saying that a function of type 'alpha' can't be applied to Bool. That's insane! And indeed users complain bitterly (#13834, #17150.) -The right error is the CHoleCan, which has /already/ been emitted by +The right error is the Hole, which has /already/ been emitted by tcUnboundId. It later reports 'wurble' as out of scope, and tries to give its type. Fortunately in tcArgs we still have access to the function, so we can check if it is a HsUnboundVar. We use this info to simply skip over any visible type arguments. We've already inferred the type of the -function, so we'll /already/ have emitted a CHoleCan constraint; +function, so we'll /already/ have emitted a Hole; failing preserves that constraint. We do /not/ want to fail altogether in this case (via failM) becuase @@ -1897,14 +1895,13 @@ tcUnboundId :: HsExpr GhcRn -> OccName -> ExpRhoType -> TcM (HsExpr GhcTc) -- Others might simply be variables that accidentally have no binding site -- -- We turn all of them into HsVar, since HsUnboundVar can't contain an --- Id; and indeed the evidence for the CHoleCan does bind it, so it's +-- Id; and indeed the evidence for the ExprHole does bind it, so it's -- not unbound any more! tcUnboundId rn_expr occ res_ty = do { ty <- newOpenFlexiTyVarTy -- Allow Int# etc (#12531) ; name <- newSysName occ ; let ev = mkLocalId name ty - ; can <- newHoleCt ExprHole ev ty - ; emitInsoluble can + ; emitNewExprHole occ ev ty ; tcWrapResultO (UnboundOccurrenceOf occ) rn_expr (HsVar noExtField (noLoc ev)) ty res_ty } diff --git a/compiler/GHC/Tc/Gen/HsType.hs b/compiler/GHC/Tc/Gen/HsType.hs index 0614bfcc95..9342c367b3 100644 --- a/compiler/GHC/Tc/Gen/HsType.hs +++ b/compiler/GHC/Tc/Gen/HsType.hs @@ -419,7 +419,7 @@ tcHsTypeApp wc_ty kind ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ A HsWildCardBndrs's hswc_ext now only includes /named/ wildcards, so any unnamed wildcards stay unchanged in hswc_body. When called in -tcHsTypeApp, tcCheckLHsType will call emitAnonWildCardHoleConstraint +tcHsTypeApp, tcCheckLHsType will call emitAnonTypeHole on these anonymous wildcards. However, this would trigger error/warning when an anonymous wildcard is passed in as a visible type argument, which we do not want because users should be able to write @@ -891,7 +891,7 @@ tcAnonWildCardOcc wc exp_kind ; warning <- woptM Opt_WarnPartialTypeSignatures ; unless (part_tysig && not warning) $ - emitAnonWildCardHoleConstraint wc_tv + emitAnonTypeHole wc_tv -- Why the 'unless' guard? -- See Note [Wildcards in visible kind application] @@ -911,11 +911,11 @@ x = MkT So we should allow '@_' without emitting any hole constraints, and regardless of whether PartialTypeSignatures is enabled or not. But how would the typechecker know which '_' is being used in VKA and which is not when it -calls emitNamedWildCardHoleConstraints in tcHsPartialSigType on all HsWildCardBndrs? +calls emitNamedTypeHole in tcHsPartialSigType on all HsWildCardBndrs? The solution then is to neither rename nor include unnamed wildcards in HsWildCardBndrs, but instead give every anonymous wildcard a fresh wild tyvar in tcAnonWildCardOcc. And whenever we see a '@', we automatically turn on PartialTypeSignatures and -turn off hole constraint warnings, and do not call emitAnonWildCardHoleConstraint +turn off hole constraint warnings, and do not call emitAnonTypeHole under these conditions. See related Note [Wildcards in visible type application] here and Note [The wildcard story for types] in GHC.Hs.Types @@ -3192,7 +3192,7 @@ tcHsPartialSigType ctxt sig_ty -- Spit out the wildcards (including the extra-constraints one) -- as "hole" constraints, so that they'll be reported if necessary -- See Note [Extra-constraint holes in partial type signatures] - ; emitNamedWildCardHoleConstraints wcs + ; mapM_ emitNamedTypeHole wcs -- Zonk, so that any nested foralls can "see" their occurrences -- See Note [Checking partial type signatures], in @@ -3365,7 +3365,7 @@ tcHsPatSigType ctxt sig_ty do { sig_ty <- tcHsOpenType hs_ty ; return (wcs, sig_ty) } - ; emitNamedWildCardHoleConstraints wcs + ; mapM_ emitNamedTypeHole wcs -- sig_ty might have tyvars that are at a higher TcLevel (if hs_ty -- contains a forall). Promote these. diff --git a/compiler/GHC/Tc/Gen/Rule.hs b/compiler/GHC/Tc/Gen/Rule.hs index 20620d2c36..708218abe5 100644 --- a/compiler/GHC/Tc/Gen/Rule.hs +++ b/compiler/GHC/Tc/Gen/Rule.hs @@ -460,9 +460,9 @@ getRuleQuantCts wc = float_wc emptyVarSet wc where float_wc :: TcTyCoVarSet -> WantedConstraints -> (Cts, WantedConstraints) - float_wc skol_tvs (WC { wc_simple = simples, wc_impl = implics }) + float_wc skol_tvs (WC { wc_simple = simples, wc_impl = implics, wc_holes = holes }) = ( simple_yes `andCts` implic_yes - , WC { wc_simple = simple_no, wc_impl = implics_no }) + , emptyWC { wc_simple = simple_no, wc_impl = implics_no, wc_holes = holes }) where (simple_yes, simple_no) = partitionBag (rule_quant_ct skol_tvs) simples (implic_yes, implics_no) = mapAccumBagL (float_implic skol_tvs) @@ -480,8 +480,6 @@ getRuleQuantCts wc | EqPred _ t1 t2 <- classifyPredType (ctPred ct) , not (ok_eq t1 t2) = False -- Note [RULE quantification over equalities] - | isHoleCt ct - = False -- Don't quantify over type holes, obviously | otherwise = tyCoVarsOfCt ct `disjointVarSet` skol_tvs -- cgit v1.2.1