[commit: ghc] master: A collection of type-inference refactorings. (3f5673f)

Simon Peyton Jones simonpj at microsoft.com
Sat Oct 22 22:23:53 UTC 2016


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 at 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 at 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


More information about the ghc-devs mailing list