RE: [commit: ghc] master: A collection of type-inference refactorings. (3f5673f)
Friends You'll have seen a brief flurry of commits from me, of which this is the biggest. I've managed to spend some time on the typechecker recently, and this is the result. I'm pleased with the results (simpler code, more predictable behaviour), but although I worked through the changes with Richard, I may have introduced new bugs, so keep an eye out. More on the way! Simon | >--------------------------------------------------------------- | | commit 3f5673f34a2f761423027bf46f64f7499708725f | Author: Simon Peyton Jones <simonpj@microsoft.com> | Date: Tue Sep 20 23:31:07 2016 +0100 | | A collection of type-inference refactorings. | | This patch does a raft of useful tidy-ups in the type checker. | I've been meaning to do this for some time, and finally made | time to do it en route to ICFP. | | 1. Modify TcType.ExpType to make a distinct data type, | InferResult for the Infer case, and consequential | refactoring. | | 2. Define a new function TcUnify.fillInferResult, to fill in | an InferResult. It uses TcMType.promoteTcType to promote | the type to the level of the InferResult. | See TcMType Note [Promoting a type] | This refactoring is in preparation for an improvement | to typechecking pattern bindings, coming next. | | I flirted with an elaborate scheme to give better | higher rank inference, but it was just too complicated. | See TcMType Note [Promotion and higher rank types] | | 3. Add to InferResult a new field ir_inst :: Bool to say | whether or not the type used to fill in the | InferResult should be deeply instantiated. See | TcUnify Note [Deep instantiation of InferResult]. | | 4. Add a TcLevel to SkolemTvs. This will be useful generally | | - it's a fast way to see if the type | variable escapes when floating (not used yet) | | - it provides a good consistency check when updating a | unification variable (TcMType.writeMetaTyVarRef, the | level_check_ok check) | | I originally had another reason (related to the flirting | in (2), but I left it in because it seems like a step in | the right direction. | | 5. Reduce and simplify the plethora of uExpType, | tcSubType and related functions in TcUnify. It was | such an opaque mess and it's still not great, but it's | better. | | 6. Simplify the uo_expected field of TypeEqOrigin. Richard | had generatlised it to a ExpType, but it was almost always | a Check type. Now it's back to being a plain TcType which | is much, much easier. | | 7. Improve error messages by refraining from skolemisation when | it's clear that there's an error: see | TcUnify Note [Don't skolemise unnecessarily] | | 8. Type.isPiTy and isForAllTy seem to be missing a coreView check, | so I added it | | 9. Kill off tcs_used_tcvs. Its purpose is to track the | givens used by wanted constraints. For dictionaries etc | we do that via the free vars of the /bindings/ in the | implication constraint ic_binds. But for coercions we | just do update-in-place in the type, rather than | generating a binding. So we need something analogous to | bindings, to track what coercions we have added. | | That was the purpose of tcs_used_tcvs. But it only | worked for a /single/ iteration, whereas we may have | multiple iterations of solving an implication. Look | at (the old) 'setImplicationStatus'. If the constraint | is unsolved, it just drops the used_tvs on the floor. | If it becomes solved next time round, we'll pick up | coercions used in that round, but ignore ones used in | the first round. | | There was an outright bug. Result = (potentialy) bogus | unused-constraint errors. Constructing a case where this | actually happens seems quite trick so I did not do so. | | Solution: expand EvBindsVar to include the (free vars of | the) coercions, so that the coercions are tracked in | essentially the same way as the bindings. | | This turned out to be much simpler. Less code, more | correct. | | 10. Make the ic_binds field in an implication have type | ic_binds :: EvBindsVar | instead of (as previously) | ic_binds :: Maybe EvBindsVar | This is notably simpler, and faster to use -- less | testing of the Maybe. But in the occaional situation | where we don't have anywhere to put the bindings, the | belt-and-braces error check is lost. So I put it back | as an ASSERT in 'setImplicationStatus' (see the use of | 'termEvidenceAllowed') | | All these changes led to quite bit of error message wibbling | | | >--------------------------------------------------------------- | | 3f5673f34a2f761423027bf46f64f7499708725f | compiler/ghci/RtClosureInspect.hs | 2 +- | compiler/typecheck/Inst.hs | 4 +- | compiler/typecheck/TcBinds.hs | 90 +-- | compiler/typecheck/TcErrors.hs | 44 +- | compiler/typecheck/TcEvidence.hs | 33 +- | compiler/typecheck/TcExpr.hs | 24 +- | compiler/typecheck/TcHsSyn.hs | 7 +- | compiler/typecheck/TcHsType.hs | 21 +- | compiler/typecheck/TcInstDcls.hs | 7 +- | compiler/typecheck/TcMType.hs | 324 ++++++--- | compiler/typecheck/TcMatches.hs | 19 +- | compiler/typecheck/TcPat.hs | 22 +- | compiler/typecheck/TcPatSyn.hs | 16 +- | compiler/typecheck/TcPluginM.hs | 16 +- | compiler/typecheck/TcRnDriver.hs | 9 +- | compiler/typecheck/TcRnMonad.hs | 28 +- | compiler/typecheck/TcRnTypes.hs | 29 +- | compiler/typecheck/TcSMonad.hs | 124 ++-- | compiler/typecheck/TcSimplify.hs | 58 +- | compiler/typecheck/TcType.hs | 78 ++- | compiler/typecheck/TcUnify.hs | 731 | +++++++++++++++------ | compiler/typecheck/TcValidity.hs | 2 +- | compiler/types/Type.hs | 2 + | compiler/vectorise/Vectorise/Generic/PData.hs | 2 +- | testsuite/tests/ado/ado004.stderr | 4 +- | .../tests/annotations/should_fail/annfail10.stderr | 12 +- | testsuite/tests/driver/T2182.stderr | 32 +- | testsuite/tests/gadt/gadt-escape1.stderr | 16 +- | testsuite/tests/gadt/gadt13.stderr | 10 +- | testsuite/tests/gadt/gadt7.stderr | 18 +- | .../tests/ghci.debugger/scripts/break012.stdout | 8 +- | .../tests/ghci.debugger/scripts/print022.stdout | 4 +- | testsuite/tests/ghci/scripts/T11524a.stdout | 4 +- | testsuite/tests/ghci/scripts/T2182ghci.stderr | 10 +- | .../tests/indexed-types/should_fail/T12386.hs | 9 + | .../tests/indexed-types/should_fail/T12386.stderr | 7 + | .../tests/indexed-types/should_fail/T5439.stderr | 16 +- | .../tests/indexed-types/should_fail/T7354.stderr | 8 +- | .../tests/parser/should_compile/read014.stderr | 2 +- | testsuite/tests/parser/should_fail/T7848.stderr | 5 +- | .../tests/parser/should_fail/readFail003.stderr | 4 +- | .../partial-sigs/should_compile/T10438.stderr | 14 +- | .../partial-sigs/should_compile/T11192.stderr | 16 +- | .../tests/patsyn/should_compile/T11213.stderr | 2 +- | testsuite/tests/patsyn/should_fail/mono.stderr | 4 +- | testsuite/tests/polykinds/T7438.stderr | 16 +- | testsuite/tests/rebindable/rebindable6.stderr | 12 +- | .../tests/rename/should_compile/T12597.stderr | 2 +- | testsuite/tests/roles/should_compile/T8958.stderr | 5 +- | .../simplCore/should_compile/noinline01.stderr | 4 +- | testsuite/tests/th/T11452.stderr | 2 +- | testsuite/tests/th/T2222.stderr | 2 +- | .../typecheck/should_compile/ExPatFail.stderr | 4 +- | .../should_compile/T12427.stderr} | 0 | .../tests/typecheck/should_compile/T12427a.stderr | 33 + | .../tests/typecheck/should_compile/tc141.stderr | 6 +- | .../tests/typecheck/should_fail/T10495.stderr | 10 +- | .../tests/typecheck/should_fail/T10619.stderr | 4 +- | .../tests/typecheck/should_fail/T12177.stderr | 19 +- | testsuite/tests/typecheck/should_fail/T3102.hs | 6 +- | testsuite/tests/typecheck/should_fail/T3102.stderr | 12 - | testsuite/tests/typecheck/should_fail/T7453.stderr | 50 +- | testsuite/tests/typecheck/should_fail/T7734.stderr | 12 +- | testsuite/tests/typecheck/should_fail/T9109.stderr | 10 +- | testsuite/tests/typecheck/should_fail/T9318.stderr | 12 +- | .../tests/typecheck/should_fail/VtaFail.stderr | 2 +- | testsuite/tests/typecheck/should_fail/all.T | 2 +- | .../tests/typecheck/should_fail/tcfail002.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail004.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail005.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail013.stderr | 2 +- | .../tests/typecheck/should_fail/tcfail014.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail018.stderr | 2 +- | .../tests/typecheck/should_fail/tcfail032.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail099.stderr | 6 +- | .../tests/typecheck/should_fail/tcfail104.stderr | 10 +- | .../tests/typecheck/should_fail/tcfail140.stderr | 4 +- | .../tests/typecheck/should_fail/tcfail181.stderr | 2 +- | .../tests/warnings/should_compile/T12574.stderr | 2 +- | 79 files changed, 1321 insertions(+), 859 deletions(-) | | Diff suppressed because of size. To see it, use: | | git diff-tree --root --patch-with-stat --no-color --find-copies- | harder --ignore-space-at-eol --cc | 3f5673f34a2f761423027bf46f64f7499708725f | _______________________________________________ | ghc-commits mailing list | ghc-commits@haskell.org | https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask | ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc- | commits&data=02%7C01%7Csimonpj%40microsoft.com%7Ccf7d693b723d4b7b061b08d3 | f9cda27f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636126634003055994& | sdata=p7fR8mA%2BXBSWTN%2B7pX6B2qs9zYM0CiXPsSI%2BvK21d8Q%3D&reserved=0
Simon Peyton Jones via ghc-devs wrote:
You'll have seen a brief flurry of commits from me, of which this is the biggest. I've managed to spend some time on the typechecker recently, and this is the result.
I'm pleased with the results (simpler code, more predictable behaviour), but although I worked through the changes with Richard, I may have introduced new bugs, so keep an eye out.
Simon, I suspect these commits may be responsible for 11 test failures: Unexpected results from: TEST="T9857 T12007 T9732 T9783 T9867 unboxed-wrapper-naked match-unboxed num T12698 T4439 unboxed-wrapper" SUMMARY for test run started at Sun Oct 23 16:56:26 2016 AEDT 0:00:02 spent to go through 11 total tests, which gave rise to 47 test cases, of which 36 were skipped 0 had missing libraries 0 expected passes 0 expected failures 0 caused framework failures 0 unexpected passes 11 unexpected failures 0 unexpected stat failures Unexpected failures: deSugar/should_compile/T4439.run T4439 [exit code non-0] (normal) ghci/scripts/T12007.run T12007 [bad stderr] (ghci) patsyn/should_compile/num.run num [exit code non-0] (normal) patsyn/should_compile/T9732.run T9732 [exit code non-0] (normal) patsyn/should_compile/T9857.run T9857 [exit code non-0] (normal) patsyn/should_compile/T9867.run T9867 [exit code non-0] (normal) patsyn/should_compile/T12698.run T12698 [exit code non-0] (normal) patsyn/should_fail/unboxed-wrapper-naked.run unboxed-wrapper-naked [stderr mismatch] (normal) patsyn/should_run/T9783.run T9783 [exit code non-0] (normal) patsyn/should_run/match-unboxed.run match-unboxed [exit code non-0] (normal) patsyn/should_run/unboxed-wrapper.run unboxed-wrapper [exit code non-0] (normal) They all fail with exactly the same callstack: ghc-stage2: panic! (the 'impossible' happened) (GHC version 8.1.20161022 for x86_64-unknown-linux): ASSERT failed! Infer{apr,2 True} :: TYPE t_apq[tau:2] a_a1pr[tau:2] Call stack: CallStack (from HasCallStack): prettyCurrentCallStack, called at compiler/utils/Outputable.hs:1076:58 in ghc:Outputable callStackDoc, called at compiler/utils/Outputable.hs:1125:22 in ghc:Outputable assertPprPanic, called at compiler/typecheck/TcUnify.hs:547:56 in ghc:TcUnify Call stack: CallStack (from HasCallStack): prettyCurrentCallStack, called at compiler/utils/Outputable.hs:1076:58 in ghc:Outputable callStackDoc, called at compiler/utils/Outputable.hs:1080:37 in ghc:Outputable pprPanic, called at compiler/utils/Outputable.hs:1123:5 in ghc:Outputable assertPprPanic, called at compiler/typecheck/TcUnify.hs:547:56 in ghc:TcUnify Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
participants (2)
-
Erik de Castro Lopo -
Simon Peyton Jones