1. 14 Feb, 2019 1 commit
    • Simon Peyton Jones's avatar
      Make a smart mkAppTyM · 68278382
      Simon Peyton Jones authored
      This patch finally delivers on Trac #15952.  Specifically
      
      * Completely remove Note [The tcType invariant], along with
        its complicated consequences (IT1-IT6).
      
      * Replace Note [The well-kinded type invariant] with:
      
            Note [The Purely Kinded Type Invariant (PKTI)]
      
      * Instead, establish the (PKTI) in TcHsType.tcInferApps,
        by using a new function mkAppTyM when building a type
        application.  See Note [mkAppTyM].
      
      * As a result we can remove the delicate mkNakedXX functions
        entirely.  Specifically, mkNakedCastTy retained lots of
        extremly delicate Refl coercions which just cluttered
        everything up, and(worse) were very vulnerable to being
        silently eliminated by (say) substTy. This led to a
        succession of bug reports.
      
      The result is noticeably simpler to explain, simpler
      to code, and Richard and I are much more confident that
      it is correct.
      
      It does not actually fix any bugs, but it brings us closer.
      E.g. I hoped it'd fix #15918 and #15799, but it doesn't quite
      do so.  However, it makes it much easier to fix.
      
      I also did a raft of other minor refactorings:
      
      * Use tcTypeKind consistently in the type checker
      
      * Rename tcInstTyBinders to tcInvisibleTyBinders,
        and refactor it a bit
      
      * Refactor tcEqType, pickyEqType, tcEqTypeVis
        Simpler, probably more efficient.
      
      * Make zonkTcType zonk TcTyCons, at least if they have
        any free unification variables -- see zonk_tc_tycon
        in TcMType.zonkTcTypeMapper.
      
        Not zonking these TcTyCons was actually a bug before.
      
      * Simplify try_to_reduce_no_cache in TcFlatten (a lot)
      
      * Combine checkExpectedKind and checkExpectedKindX.
        And then combine the invisible-binder instantation code
        Much simpler now.
      
      * Fix a little bug in TcMType.skolemiseQuantifiedTyVar.
        I'm not sure how I came across this originally.
      
      * Fix a little bug in TyCoRep.isUnliftedRuntimeRep
        (the ASSERT was over-zealous).  Again I'm not certain
        how I encountered this.
      
      * Add a missing solveLocalEqualities in
        TcHsType.tcHsPartialSigType.
        I came across this when trying to get level numbers
        right.
      68278382
  2. 12 Feb, 2019 1 commit
    • Richard Eisenberg's avatar
      Fix #16188 · 4a4ae70f
      Richard Eisenberg authored
      There was an awful lot of zipping going on in
      canDecomposableTyConAppOK, and one of the lists being zipped
      was too short, causing the result to be too short. Easily
      fixed.
      
      Also fixes #16204 and #16225
      
      test case: typecheck/should_compile/T16188
                 typecheck/should_compile/T16204[ab]
                 typecheck/should_fail/T16204c
                 typecheck/should_compile/T16225
      4a4ae70f
  3. 08 Feb, 2019 1 commit
    • Richard Eisenberg's avatar
      Fix #14729 by making the normaliser homogeneous · 2b90356d
      Richard Eisenberg authored
      This ports the fix to #12919 to the normaliser. (#12919 was about
      the flattener.) Because the fix is involved, this is done by
      moving the critical piece of code to Coercion, and then calling
      this from both the flattener and the normaliser.
      
      The key bit is: simplifying type families in a type is always
      a *homogeneous* operation. See #12919 for a discussion of why
      this is the Right Way to simplify type families.
      
      Also fixes #15549.
      
      test case: dependent/should_compile/T14729{,kind}
                 typecheck/should_compile/T15549[ab]
      2b90356d
  4. 07 Dec, 2018 1 commit
    • Simon Peyton Jones's avatar
      Wibble to Taming the Kind Inference Monster · 5b7ca039
      Simon Peyton Jones authored
      I had allowed rename/should_fail/T15828 (Trac #15828) to regress a bit.
      The main payload of this patch is to fix that problem, at the cost of
      more contortions in checkConsistentFamInst.  Oh well, at least they are
      highly localised.
      
      I also update the -ddump-types code in TcRnDriver to print out some
      more expicit information about each type constructor, thus instead of
      
         DF{3} :: forall k. * -> k -> *
      
      we get
      
         data family DF{3} :: forall k. * -> k -> *
      
      Remember, this is debug-printing only.  This change is the reason
      that so many .stderr files change.
      5b7ca039
  5. 29 Nov, 2018 1 commit
    • Simon Peyton Jones's avatar
      Taming the Kind Inference Monster · 2257a86d
      Simon Peyton Jones authored
      My original goal was (Trac #15809) to move towards using level numbers
      as the basis for deciding which type variables to generalise, rather
      than searching for the free varaibles of the environment.  However
      it has turned into a truly major refactoring of the kind inference
      engine.
      
      Let's deal with the level-numbers part first:
      
      * Augment quantifyTyVars to calculate the type variables to
        quantify using level numbers, and compare the result with
        the existing approach.  That is; no change in behaviour,
        just a WARNing if the two approaches give different answers.
      
      * To do this I had to get the level number right when calling
        quantifyTyVars, and this entailed a bit of care, especially
        in the code for kind-checking type declarations.
      
      * However, on the way I was able to eliminate or simplify
        a number of calls to solveEqualities.
      
      This work is incomplete: I'm not /using/ level numbers yet.
      When I subsequently get rid of any remaining WARNings in
      quantifyTyVars, that the level-number answers differ from
      the current answers, then I can rip out the current
      "free vars of the environment" stuff.
      
      Anyway, this led me into deep dive into kind inference for type and
      class declarations, which is an increasingly soggy part of GHC.
      Richard already did some good work recently in
      
         commit 5e45ad10
         Date:   Thu Sep 13 09:56:02 2018 +0200
      
          Finish fix for #14880.
      
          The real change that fixes the ticket is described in
          Note [Naughty quantification candidates] in TcMType.
      
      but I kept turning over stones. So this patch has ended up
      with a pretty significant refactoring of that code too.
      
      Kind inference for types and classes
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      
      * Major refactoring in the way we generalise the inferred kind of
        a TyCon, in kcTyClGroup.  Indeed, I made it into a new top-level
        function, generaliseTcTyCon.  Plus a new Note to explain it
        Note [Inferring kinds for type declarations].
      
      * We decided (Trac #15592) not to treat class type variables specially
        when dealing with Inferred/Specified/Required for associated types.
        That simplifies things quite a bit. I also rewrote
        Note [Required, Specified, and Inferred for types]
      
      * Major refactoring of the crucial function kcLHsQTyVars:
        I split it into
             kcLHsQTyVars_Cusk  and  kcLHsQTyVars_NonCusk
        because the two are really quite different. The CUSK case is
        almost entirely rewritten, and is much easier because of our new
        decision not to treat the class variables specially
      
      * I moved all the error checks from tcTyClTyVars (which was a bizarre
        place for it) into generaliseTcTyCon and/or the CUSK case of
        kcLHsQTyVars.  Now tcTyClTyVars is extremely simple.
      
      * I got rid of all the all the subtleties in tcImplicitTKBndrs. Indeed
        now there is no difference between tcImplicitTKBndrs and
        kcImplicitTKBndrs; there is now a single bindImplicitTKBndrs.
        Same for kc/tcExplicitTKBndrs.  None of them monkey with level
        numbers, nor build implication constraints.  scopeTyVars is gone
        entirely, as is kcLHsQTyVarBndrs. It's vastly simpler.
      
        I found I could get rid of kcLHsQTyVarBndrs entirely, in favour of
        the bnew bindExplicitTKBndrs.
      
      Quantification
      ~~~~~~~~~~~~~~
      * I now deal with the "naughty quantification candidates"
        of the previous patch in candidateQTyVars, rather than in
        quantifyTyVars; see Note [Naughty quantification candidates]
        in TcMType.
      
        I also killed off closeOverKindsCQTvs in favour of the same
        strategy that we use for tyCoVarsOfType: namely, close over kinds
        at the occurrences.
      
        And candidateQTyVars no longer needs a gbl_tvs argument.
      
      * Passing the ContextKind, rather than the expected kind itself,
        to tc_hs_sig_type_and_gen makes it easy to allocate the expected
        result kind (when we are in inference mode) at the right level.
      
      Type families
      ~~~~~~~~~~~~~~
      * I did a major rewrite of the impenetrable tcFamTyPats. The result
        is vastly more comprehensible.
      
      * I got rid of kcDataDefn entirely, quite a big function.
      
      * I re-did the way that checkConsistentFamInst works, so
        that it allows alpha-renaming of invisible arguments.
      
      * The interaction of kind signatures and family instances is tricky.
          Type families: see Note [Apparently-nullary families]
          Data families: see Note [Result kind signature for a data family instance]
                         and Note [Eta-reduction for data families]
      
      * The consistent instantation of an associated type family is tricky.
        See Note [Checking consistent instantiation] and
            Note [Matching in the consistent-instantation check]
        in TcTyClsDecls.  It's now checked in TcTyClsDecls because that is
        when we have the relevant info to hand.
      
      * I got tired of the compromises in etaExpandFamInst, so I did the
        job properly by adding a field cab_eta_tvs to CoAxBranch.
        See Coercion.etaExpandCoAxBranch.
      
      tcInferApps and friends
      ~~~~~~~~~~~~~~~~~~~~~~~
      * I got rid of the mysterious and horrible ClsInstInfo argument
        to tcInferApps, checkExpectedKindX, and various checkValid
        functions.  It was horrible!
      
      * I got rid of [Type] result of tcInferApps.  This list was used
        only in tcFamTyPats, when checking the LHS of a type instance;
        and if there is a cast in the middle, the list is meaningless.
        So I made tcInferApps simpler, and moved the complexity
        (not much) to tcInferApps.
      
        Result: tcInferApps is now pretty comprehensible again.
      
      * I refactored the many function in TcMType that instantiate skolems.
      
      Smaller things
      
      * I rejigged the error message in checkValidTelescope; I think it's
        quite a bit better now.
      
      * checkValidType was not rejecting constraints in a kind signature
           forall (a :: Eq b => blah). blah2
        That led to further errors when we then do an ambiguity check.
        So I make checkValidType reject it more aggressively.
      
      * I killed off quantifyConDecl, instead calling kindGeneralize
        directly.
      
      * I fixed an outright bug in tyCoVarsOfImplic, where we were not
        colleting the tyvar of the kind of the skolems
      
      * Renamed ClsInstInfo to AssocInstInfo, and made it into its
        own data type
      
      * Some fiddling around with pretty-printing of family
        instances which was trickier than I thought.  I wanted
        wildcards to print as plain "_" in user messages, although
        they each need a unique identity in the CoAxBranch.
      
      Some other oddments
      
      * Refactoring around the trace messages from reportUnsolved.
      * A bit of extra tc-tracing in TcHsSyn.commitFlexi
      
      This patch fixes a raft of bugs, and includes tests for them.
      
       * #14887
       * #15740
       * #15764
       * #15789
       * #15804
       * #15817
       * #15870
       * #15874
       * #15881
      2257a86d
  6. 22 Nov, 2018 1 commit
    • Ryan Scott's avatar
      Fix #15852 by eta expanding data family instance RHSes, too · 014d6c1f
      Ryan Scott authored
      When I defined `etaExpandFamInstLHS`, I blatantly forgot
      to eta expand the RHSes of data family instances. (Actually, I
      claimed that they didn't //need// to be eta expanded. I'm not sure
      what I was thinking.)
      
      This fixes the issue by changing `etaExpandFamInstLHS` to
      `etaExpandFamInst` and, well, making it actually eta expand the RHS.
      
      Test Plan: make test TEST=T15852
      
      Reviewers: goldfire, bgamari
      
      Reviewed By: goldfire
      
      Subscribers: rwbarton, carter
      
      GHC Trac Issues: #15852
      
      Differential Revision: https://phabricator.haskell.org/D5328
      014d6c1f
  7. 08 Nov, 2018 1 commit
    • Ryan Scott's avatar
      Fix #15845 by defining etaExpandFamInstLHS and using it · 63a81707
      Ryan Scott authored
      Summary:
      Both #9692 and #14179 were caused by GHC being careless
      about using eta-reduced data family instance axioms. Each of those
      tickets were fixed by manually whipping up some code to eta-expand
      the axioms. The same sort of issue has now caused #15845, so I
      figured it was high time to factor out the code that each of these
      fixes have in common.
      
      This patch introduces the `etaExpandFamInstLHS` function, which takes
      a family instance's type variables, LHS types, and RHS type, and
      returns type variables and LHS types that have been eta-expanded if
      necessary, in the case of a data family instance. (If it's a type
      family instance, `etaExpandFamInstLHS` just returns the supplied type
      variables and LHS types unchanged).
      
      Along the way, I noticed that many references to
      `Note [Eta reduction for data families]` (in `FamInstEnv`) had
      slightly bitrotted (they either referred to a somewhat different
      name, or claimed that the Note lived in a different module), so
      I took the liberty of cleaning those up.
      
      Test Plan: make test TEST="T9692 T15845"
      
      Reviewers: goldfire, bgamari
      
      Reviewed By: goldfire
      
      Subscribers: rwbarton, carter
      
      GHC Trac Issues: #15845
      
      Differential Revision: https://phabricator.haskell.org/D5294
      63a81707
  8. 22 Oct, 2018 1 commit
  9. 19 Oct, 2018 1 commit
    • Ningning Xie's avatar
      Adding almost devoid check for covar in ForAllCo · 879db559
      Ningning Xie authored
      Summary:
      For the sake of consistency of the dependent core, there is a restriction on
      where a coercion variable can appear in ForAllCo: the coercion variable can
      appear nowhere except in coherence coercions.
      
      Currently this restriction is missing in Core. The goal of this patch is to add
      the missing restriction.
      
      After discussion, we decide: coercion variables can appear nowhere except in
      `GRefl` and `Refl`. Relaxing the restriction to include `Refl` should not break
      consistency, we premuse.
      
      Test Plan: ./validate
      
      Reviewers: goldfire, simonpj, bgamari
      
      Reviewed By: goldfire
      
      Subscribers: rwbarton, carter
      
      GHC Trac Issues: #15757
      
      Differential Revision: https://phabricator.haskell.org/D5231
      879db559
  10. 15 Sep, 2018 1 commit
    • Ningning Xie's avatar
      Coercion Quantification · ea5ade34
      Ningning Xie authored
      This patch corresponds to #15497.
      
      According to https://ghc.haskell.org/trac/ghc/wiki/DependentHaskell/Phase2,
       we would like to have coercion quantifications back. This will
      allow us to migrate (~#) to be homogeneous, instead of its current
      heterogeneous definition. This patch is (lots of) plumbing only. There
      should be no user-visible effects.
      
      An overview of changes:
      
      - Both `ForAllTy` and `ForAllCo` can quantify over coercion variables,
      but only in *Core*. All relevant functions are updated accordingly.
      - Small changes that should be irrelevant to the main task:
          1. removed dead code `mkTransAppCo` in Coercion
          2. removed out-dated Note Computing a coercion kind and
             roles in Coercion
          3. Added `Eq4` in Note Respecting definitional equality in
             TyCoRep, and updated `mkCastTy` accordingly.
          4. Various updates and corrections of notes and typos.
      - Haddock submodule needs to be changed too.
      
      Acknowledgments:
      This work was completed mostly during Ningning Xie's Google Summer
      of Code, sponsored by Google. It was advised by Richard Eisenberg,
      supported by NSF grant 1704041.
      
      Test Plan: ./validate
      
      Reviewers: goldfire, simonpj, bgamari, hvr, erikd, simonmar
      
      Subscribers: RyanGlScott, monoidal, rwbarton, carter
      
      GHC Trac Issues: #15497
      
      Differential Revision: https://phabricator.haskell.org/D5054
      ea5ade34
  11. 21 Aug, 2018 1 commit
    • Andreas Klebinger's avatar
      Replace most occurences of foldl with foldl'. · 09c1d5af
      Andreas Klebinger authored
      This patch adds foldl' to GhcPrelude and changes must occurences
      of foldl to foldl'. This leads to better performance especially
      for quick builds where GHC does not perform strictness analysis.
      
      It does change strictness behaviour when we use foldl' to turn
      a argument list into function applications. But this is only a
      drawback if code looks ONLY at the last argument but not at the first.
      And as the benchmarks show leads to fewer allocations in practice
      at O2.
      
      Compiler performance for Nofib:
      
      O2 Allocations:
              -1 s.d.                -----            -0.0%
              +1 s.d.                -----            -0.0%
              Average                -----            -0.0%
      
      O2 Compile Time:
              -1 s.d.                -----            -2.8%
              +1 s.d.                -----            +1.3%
              Average                -----            -0.8%
      
      O0 Allocations:
              -1 s.d.                -----            -0.2%
              +1 s.d.                -----            -0.1%
              Average                -----            -0.2%
      
      Test Plan: ci
      
      Reviewers: goldfire, bgamari, simonmar, tdammers, monoidal
      
      Reviewed By: bgamari, monoidal
      
      Subscribers: tdammers, rwbarton, thomie, carter
      
      Differential Revision: https://phabricator.haskell.org/D4929
      09c1d5af
  12. 23 Jul, 2018 1 commit
    • Richard Eisenberg's avatar
      Fix some casts. · af624071
      Richard Eisenberg authored
      This fixes #15346, and is a team effort between Ryan Scott and
      myself (mostly Ryan). We discovered two errors related to FC's
      "push" rules, one in the TPush rule (as implemented in pushCoTyArg)
      and one in KPush rule (it shows up in liftCoSubstVarBndr).
      
      The solution: do what the paper says, instead of whatever random
      thoughts popped into my head as I was actually implementing.
      
      Also fixes #15419, which is actually the same underlying problem.
      
      Test case: dependent/should_compile/T{15346,15419}.
      af624071
  13. 10 Jul, 2018 2 commits
    • Simon Peyton Jones's avatar
      Fix decompsePiCos and visible type application · aedbf7f1
      Simon Peyton Jones authored
      Trac #15343 was caused by two things
      
      First, in TcHsType.tcHsTypeApp, which deals with the type argment
      in visible type application, we were failing to call
      solveLocalEqualities. But the type argument is like a user type
      signature so it's at least inconsitent not to do so.
      
      I thought that would nail it.  But it didn't. It turned out that we
      were ended up calling decomposePiCos on a type looking like this
          (f |> co) Int
      
      where co :: (forall a. ty) ~ (t1 -> t2)
      
      Now, 'co' is insoluble, and we'll report that later.  But meanwhile
      we don't want to crash in decomposePiCos.
      
      My fix involves keeping track of the type on both sides of the
      coercion, and ensuring that the outer shape matches before
      decomposing.  I wish there was a simpler way to do this. But
      I think this one is at least robust.
      
      I suppose it is possible that the decomposePiCos fix would
      have cured the original report, but I'm leaving the one-line
      tcHsTypeApp fix in too because it just seems more consistent.
      aedbf7f1
    • Ningning Xie's avatar
      Refactor coercion rule · 55a3f855
      Ningning Xie authored
      Summary:
      The patch is an attempt on #15192.
      
      It defines a new coercion rule
      
      ```
       | GRefl Role Type MCoercion
      ```
      
      which correspondes to the typing rule
      
      ```
           t1 : k1
        ------------------------------------
        GRefl r t1 MRefl: t1 ~r t1
      
           t1 : k1       co :: k1 ~ k2
        ------------------------------------
        GRefl r t1 (MCo co) : t1 ~r t1 |> co
      ```
      
      MCoercion wraps a coercion, which might be reflexive (MRefl)
      or not (MCo co). To know more about MCoercion see #14975.
      
      We keep Refl ty as a special case for nominal reflexive coercions,
      naemly, Refl ty :: ty ~n ty.
      
      This commit is meant to be a general performance improvement,
      but there are a few regressions. See #15192, comment:13 for
      more information.
      
      Test Plan: ./validate
      
      Reviewers: bgamari, goldfire, simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #15192
      
      Differential Revision: https://phabricator.haskell.org/D4747
      55a3f855
  14. 29 Jun, 2018 1 commit
  15. 18 Jun, 2018 1 commit
    • Simon Peyton Jones's avatar
      Two small refactorings · 850ae8c5
      Simon Peyton Jones authored
      * Define Type.substTyVarBndrs, and use it
      
      * Rename substTyVarBndrCallback to substTyVarBndrUsing,
        and other analogous higher order functions.  I kept
        stumbling over the name.
      850ae8c5
  16. 30 May, 2018 1 commit
    • Ningning Xie's avatar
      Define MCoercion type · 9aac442f
      Ningning Xie authored
      An attempt on #14975:
      During compilation, reflexive casts is discarded for computation.
      Currently in some places we use Maybe coercion as inputs. So if a cast
      is reflexive it is denoted as Nothing, otherwise Just coercion.
      
      This patch defines the type
      
      data MCoercion = MRefl | MCo Coercion
      
      which is isomorphic to Maybe Coercion but useful in a number of places,
      and super-helpful documentation.
      
      Test Plan: validate
      
      Reviewers: bgamari, goldfire, simonpj
      
      Reviewed By: goldfire
      
      Subscribers: mpickering, rwbarton, thomie, carter
      
      GHC Trac Issues: #14975
      
      Differential Revision: https://phabricator.haskell.org/D4699
      9aac442f
  17. 24 May, 2018 1 commit
    • Ryan Scott's avatar
      Clean up the conflicting data family instances error message · 979f085c
      Ryan Scott authored
      Summary:
      The way we were pretty-printing conflicting data family
      instances in an error message was far from ideal:
      
      1. If a data type had no constructors, it would print an equals sign
         with nothing to the right of it.
      2. It would try to print GADTs using Haskell98 syntax.
      3. It eta-reduced away some type variables from the LHS.
      
      This patch addresses these three issues:
      
      1. We no longer print constructors at all in this error message.
         There's really no reason to do so in the first place, since
         duplicate data family instances always conflict, regardless of
         their constructors.
      2. Since we no longer print constructors, we no longer have to
         worry about whether we're using GADT or Haskell98 syntax.
      3. I've put in a fix to ensure that type variables are no longer
         eta-reduced away from the LHS.
      
      Test Plan: make test TEST=T14179
      
      Reviewers: goldfire, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14179
      
      Differential Revision: https://phabricator.haskell.org/D4711
      979f085c
  18. 23 May, 2018 1 commit
    • Simon Peyton Jones's avatar
      Add missing check to isReflCoVar_maybe · 86bba7d5
      Simon Peyton Jones authored
      isReflCoVar_maybe is called, by CoreLint, on all sorts of
      Vars (tyvars, term vars, coercion vars).  But it was silently
      assuming that it was always called on a CoVar, and as a result
      could crash fatally.  This is the immediate cause of the panic
      in Trac #15163.
      
      It's easy to fix.
      
      NB: this does not completely fix Trac #15163; more to come
      86bba7d5
  19. 23 Apr, 2018 1 commit
  20. 20 Apr, 2018 1 commit
    • Tobias Dammers's avatar
      Caching coercion roles in NthCo and coercionKindsRole refactoring · 2fbe0b51
      Tobias Dammers authored
      While addressing nonlinear behavior related to coercion roles,
      particularly `NthCo`, we noticed that coercion roles are recalculated
      often even though they should be readily at hand already in most cases.
      This patch adds a `Role` to the `NthCo` constructor so that we can cache
      them rather than having to recalculate them on the fly.
      https://ghc.haskell.org/trac/ghc/ticket/11735#comment:23 explains the
      approach.
      
      Performance improvement over GHC HEAD, when compiling Grammar.hs (see below):
      
      GHC 8.2.1:
      ```
      ghc Grammar.hs  176.27s user 0.23s system 99% cpu 2:56.81 total
      ```
      
      before patch (but with other optimizations applied):
      ```
      ghc Grammar.hs -fforce-recomp  175.77s user 0.19s system 100% cpu 2:55.78 total
      ```
      
      after:
      ```
      ../../ghc/inplace/bin/ghc-stage2 Grammar.hs  10.32s user 0.17s system 98% cpu 10.678 total
      ```
      
      Introduces the following regressions:
      
      - perf/compiler/parsing001 (possibly false positive)
      - perf/compiler/T9872
      - perf/compiler/haddock.base
      
      Reviewers: goldfire, bgamari, simonpj
      
      Reviewed By: simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #11735
      
      Differential Revision: https://phabricator.haskell.org/D4394
      2fbe0b51
  21. 13 Apr, 2018 1 commit
  22. 01 Apr, 2018 1 commit
    • Richard Eisenberg's avatar
      Track type variable scope more carefully. · faec8d35
      Richard Eisenberg authored
      The main job of this commit is to track more accurately the scope
      of tyvars introduced by user-written foralls. For example, it would
      be to have something like this:
      
        forall a. Int -> (forall k (b :: k). Proxy '[a, b]) -> Bool
      
      In that type, a's kind must be k, but k isn't in scope. We had a
      terrible way of doing this before (not worth repeating or describing
      here, but see the old tcImplicitTKBndrs and friends), but now
      we have a principled approach: make an Implication when kind-checking
      a forall. Doing so then hooks into the existing machinery for
      preventing skolem-escape, performing floating, etc. This also means
      that we bump the TcLevel whenever going into a forall.
      
      The new behavior is done in TcHsType.scopeTyVars, but see also
      TcHsType.tc{Im,Ex}plicitTKBndrs, which have undergone significant
      rewriting. There are several Notes near there to guide you. Of
      particular interest there is that Implication constraints can now
      have skolems that are out of order; this situation is reported in
      TcErrors.
      
      A major consequence of this is a slightly tweaked process for type-
      checking type declarations. The new Note [Use SigTvs in kind-checking
      pass] in TcTyClsDecls lays it out.
      
      The error message for dependent/should_fail/TypeSkolEscape has become
      noticeably worse. However, this is because the code in TcErrors goes to
      some length to preserve pre-8.0 error messages for kind errors. It's time
      to rip off that plaster and get rid of much of the kind-error-specific
      error messages. I tried this, and doing so led to a lovely error message
      for TypeSkolEscape. So: I'm accepting the error message quality regression
      for now, but will open up a new ticket to fix it, along with a larger
      error-message improvement I've been pondering. This applies also to
      dependent/should_fail/{BadTelescope2,T14066,T14066e}, polykinds/T11142.
      
      Other minor changes:
       - isUnliftedTypeKind didn't look for tuples and sums. It does now.
      
       - check_type used check_arg_type on both sides of an AppTy. But the left
         side of an AppTy isn't an arg, and this was causing a bad error message.
         I've changed it to use check_type on the left-hand side.
      
       - Some refactoring around when we print (TYPE blah) in error messages.
         The changes decrease the times when we do so, to good effect.
         Of course, this is still all controlled by
         -fprint-explicit-runtime-reps
      
      Fixes #14066 #14749
      
      Test cases: dependent/should_compile/{T14066a,T14749},
                  dependent/should_fail/T14066{,c,d,e,f,g,h}
      faec8d35
  23. 26 Mar, 2018 2 commits
    • alexvieth's avatar
      Fix performance of flattener patch (#12919) · b47a6c3a
      alexvieth authored
      This patch, authored by alexvieth and reviewed at D4451,
      makes performance improvements by critically optimizing parts
      of the flattener.
      
      Summary:
      T3064, T5321FD, T5321Fun, T9872a, T9872b, T9872c all pass.
      T9872a and T9872c show improvements beyond the -5% threshold.
      T9872d fails at 10.9% increased allocations.
      b47a6c3a
    • Richard Eisenberg's avatar
      Fix #12919 by making the flattener homegeneous. · e3dbb44f
      Richard Eisenberg authored
      This changes a key invariant of the flattener. Previously,
      flattening a type meant flattening its kind as well. But now,
      flattening is always homogeneous -- that is, the kind of the
      flattened type is the same as the kind of the input type.
      This is achieved by various wizardry in the TcFlatten.flatten_many
      function, as described in Note [flatten_many].
      
      There are several knock-on effects, including some refactoring
      in the canonicalizer to take proper advantage of the flattener's
      changed behavior. In particular, the tyvar case of can_eq_nc' no
      longer needs to take casts into account.
      
      Another effect is that flattening a tyconapp might change it
      into a casted tyconapp. This might happen if the result kind
      of the tycon contains a variable, and that variable changes
      during flattening. Because the flattener is homogeneous, it tacks
      on a cast to keep the tyconapp kind the same. However, this
      is problematic when flattening CFunEqCans, which need to have
      an uncasted tyconapp on the LHS and must remain homogeneous.
      The solution is a more involved canCFunEqCan, described in
      Note [canCFunEqCan].
      
      This patch fixes #13643 (as tested in typecheck/should_compile/T13643)
      and the panic in typecheck/should_compile/T13822 (as reported in #14024).
      Actually, there were two bugs in T13822: the first was just some
      incorrect logic in tryFill (part of the unflattener) -- also fixed
      in this patch -- and the other was the main bug fixed in this ticket.
      
      The changes in this patch exposed a long-standing flaw in OptCoercion,
      in that breaking apart an AppCo sometimes has unexpected effects on
      kinds. See new Note [EtaAppCo] in OptCoercion, which explains the
      problem and fix.
      
      Also here is a reversion of the major change in
      09bf135a, affecting ctEvCoercion.
      It turns out that making the flattener homogeneous changes the
      invariants on the algorithm, making the change in that patch
      no longer necessary.
      
      This patch also fixes:
        #14038 (dependent/should_compile/T14038)
        #13910 (dependent/should_compile/T13910)
        #13938 (dependent/should_compile/T13938)
        #14441 (typecheck/should_compile/T14441)
        #14556 (dependent/should_compile/T14556)
        #14720 (dependent/should_compile/T14720)
        #14749 (typecheck/should_compile/T14749)
      
      Sadly, this patch negatively affects performance of type-family-
      heavy code. The following patch fixes these performance degradations.
      However, the performance fixes are somewhat invasive and so I've
      kept them as a separate patch, labeling this one as [skip ci] so
      that validation doesn't fail on the performance cases.
      e3dbb44f
  24. 01 Feb, 2018 1 commit
  25. 25 Jan, 2018 1 commit
  26. 03 Jan, 2018 1 commit
  27. 21 Dec, 2017 1 commit
    • Simon Peyton Jones's avatar
      Refactor coercion holes · a492af06
      Simon Peyton Jones authored
      In fixing Trac #14584 I found that it would be /much/ more
      convenient if a "hole" in a coercion (much like a unification
      variable in a type) acutally had a CoVar associated with it
      rather than just a Unique.  Then I can ask what the free variables
      of a coercion is, and get a set of CoVars including those
      as-yet-un-filled in holes.
      
      Once that is done, it makes no sense to stuff coercion holes
      inside UnivCo.  They were there before so we could know the
      kind and role of a "hole" coercion, but once there is a CoVar
      we can get that info from the CoVar.  So I removed HoleProv
      from UnivCoProvenance and added HoleCo to Coercion.
      
      In summary:
      
      * Add HoleCo to Coercion and remove HoleProv from UnivCoProvanance
      
      * Similarly in IfaceCoercion
      
      * Make CoercionHole have a CoVar in it, not a Unique
      
      * Make tyCoVarsOfCo return the free coercion-hole variables
        as well as the ordinary free CoVars.  Similarly, remember
        to zonk the CoVar in a CoercionHole
      
      We could go further, and remove CoercionHole as a distinct type
      altogther, just collapsing it into HoleCo.  But I have not done
      that yet.
      a492af06
  28. 19 Sep, 2017 1 commit
    • Herbert Valerio Riedel's avatar
      compiler: introduce custom "GhcPrelude" Prelude · f63bc730
      Herbert Valerio Riedel authored
      This switches the compiler/ component to get compiled with
      -XNoImplicitPrelude and a `import GhcPrelude` is inserted in all
      modules.
      
      This is motivated by the upcoming "Prelude" re-export of
      `Semigroup((<>))` which would cause lots of name clashes in every
      modulewhich imports also `Outputable`
      
      Reviewers: austin, goldfire, bgamari, alanz, simonmar
      
      Reviewed By: bgamari
      
      Subscribers: goldfire, rwbarton, thomie, mpickering, bgamari
      
      Differential Revision: https://phabricator.haskell.org/D3989
      f63bc730
  29. 12 Aug, 2017 1 commit
  30. 27 Jul, 2017 1 commit
  31. 02 Jun, 2017 1 commit
    • Ryan Scott's avatar
      Use lengthIs and friends in more places · a786b136
      Ryan Scott authored
      While investigating #12545, I discovered several places in the code
      that performed length-checks like so:
      
      ```
      length ts == 4
      ```
      
      This is not ideal, since the length of `ts` could be much longer than 4,
      and we'd be doing way more work than necessary! There are already a slew
      of helper functions in `Util` such as `lengthIs` that are designed to do
      this efficiently, so I found every place where they ought to be used and
      did just that. I also defined a couple more utility functions for list
      length that were common patterns (e.g., `ltLength`).
      
      Test Plan: ./validate
      
      Reviewers: austin, hvr, goldfire, bgamari, simonmar
      
      Reviewed By: bgamari, simonmar
      
      Subscribers: goldfire, rwbarton, thomie
      
      Differential Revision: https://phabricator.haskell.org/D3622
      a786b136
  32. 29 Mar, 2017 2 commits
    • Simon Peyton Jones's avatar
      Add a couple of HasDebugCallStack contexts · 60d338f5
      Simon Peyton Jones authored
      Just for future (and past) debugging...
      60d338f5
    • Simon Peyton Jones's avatar
      Allow unbound Refl binders in a RULE · 8674883c
      Simon Peyton Jones authored
      Trac #13410 was failing because we had a RULE with a binder
         (c :: t~t)
      and the /occurrences/ of c on the LHS were being optimised to Refl,
      leaving a binder that would not be filled in by matching the LHS
      of the rule.
      
      I flirted with trying to ensure that occurrences (c :: t~t) are
      not optimised to Relf, but that turned out to be fragile; it was
      being done, for good reasons, in multiple places, including
        - TyCoRep.substCoVarBndr
        - Simplify.simplCast
        - Corecion.mkCoVarCo
      
      So I fixed it in one place by making Rules.matchN deal happily
      with an unbound binder (c :: t~t).  Quite easy.  See "Coercion
      variables" in Note [Unbound RULE binders] in Rules.
      
      In addition, I needed to make CoreLint be happy with an bound
      RULE binder that is a Relf coercion variable
      
      In debugging this, I was perplexed that occurrences of a variable
      (c :: t~t) mysteriously turned into Refl.  I found out how it
      was happening, and decided to move it:
      
      * In TyCoRep.substCoVarBndr, do not substitute Refl for a
        binder (c :: t~t).
      
      * In mkCoVarCo do not optimise (c :: t~t) to Refl.
      
      Instead, we do this optimisation in optCoercion (specifically
      opt_co4) where, surprisingly, the optimisation was /not/
      being done.  This has no effect on what programs compile;
      it just moves a relatively-expensive optimisation to optCoercion,
      where it seems more properly to belong.  It's actually not clear
      to me which is really "better", but this way round is less
      surprising.
      
      One small simplifying refactoring
      
      * Eliminate TyCoRep.substCoVarBndrCallback, which was only
        called locally.
      8674883c
  33. 28 Feb, 2017 1 commit
  34. 26 Feb, 2017 1 commit
    • Ben Gamari's avatar
      Coercion: Try dropping constraintIsLifted axiom · d2f48495
      Ben Gamari authored
      While working through the FunCo patch I encountered some lint issues
      which suggested that `Constraint` wasn't being considered equal to `TYPE
      'LiftedRep`.  Consequently I introduced this axiom and associated messy
      ball of logic to explicitly coerce `Constraint`.
      
      However, as @goldfire points out on D3208 this really shouldn't be
      necessary.  Indeed, I tried ripping out the axiom and things appear to
      just work. I suspect the issue motivating the axiom was a bug elsewhere
      in the FunCo branch that I fixed during development.
      
      Test Plan: Validate
      
      Reviewers: simonpj, goldfire, austin
      
      Reviewed By: goldfire
      
      Subscribers: thomie, goldfire
      
      Differential Revision: https://phabricator.haskell.org/D3218
      d2f48495
  35. 18 Feb, 2017 1 commit
    • Ben Gamari's avatar
      Generalize kind of the (->) tycon · b207b536
      Ben Gamari authored
      This is generalizes the kind of `(->)`, as discussed in #11714.
      
      This involves a few things,
      
       * Generalizing the kind of `funTyCon`, adding two new `RuntimeRep`
      binders,
        ```lang=haskell
      (->) :: forall (r1 :: RuntimeRep) (r2 :: RuntimeRep)
                     (a :: TYPE r1) (b :: TYPE r2).
              a -> b -> *
        ```
      
       * Unsaturated applications of `(->)` are expressed as explicit
      `TyConApp`s
      
       * Saturated applications of `(->)` are expressed as `FunTy` as they are
      currently
      
       * Saturated applications of `(->)` are expressed by a new `FunCo`
      constructor in coercions
      
       * `splitTyConApp` needs to ensure that `FunTy`s are split to a
      `TyConApp`
         of `(->)` with the appropriate `RuntimeRep` arguments
      
       * Teach CoreLint to check that all saturated applications of `(->)` are
      represented with `FunTy`
      
      At the moment I assume that `Constraint ~ *`, which is an annoying
      source of complexity. This will
      be simplified once D3023 is resolved.
      
      Also, this introduces two known regressions,
      
      `tcfail181`, `T10403`
      =====================
      Only shows the instance,
      
          instance Monad ((->) r) -- Defined in ‘GHC.Base’
      
      in its error message when -fprint-potential-instances is used. This is
      because its instance head now mentions 'LiftedRep which is not in scope.
      I'm not entirely sure of the right way to fix this so I'm just accepting
      the new output for now.
      
      T5963 (Typeable)
      ================
      
      T5963 is now broken since Data.Typeable.Internals.mkFunTy computes its
      fingerprint without the RuntimeRep variables that (->) expects. This
      will be fixed with the merge of D2010.
      
      Haddock performance
      ===================
      
      The `haddock.base` and `haddock.Cabal` tests regress in allocations by
      about 20%. This certainly hurts, but it's also not entirely unexpected:
      the size of every function type grows with this patch and Haddock has a
      lot of functions in its heap.
      b207b536
  36. 23 Jan, 2017 1 commit
  37. 06 Jan, 2017 1 commit
    • Simon Peyton Jones's avatar
      Fix the implementation of the "push rules" · b4f2afe7
      Simon Peyton Jones authored
      Richard pointed out (comment:12 of Trac #13025) that my
      implementation of the coercion "push rules", newly added
      in exprIsConAppMaybe by commit b4c3a668, wasn't quite right.
      
      But in fact that means that the implementation of those same
      rules in Simplify.simplCast was wrong too.
      
      Hence this commit:
      
      * Refactor the push rules so they are implemented in just
        one place (CoreSubst.pushCoArgs, pushCoTyArg, pushCoValArg)
        The code in Simplify gets simpler, which is nice.
      
      * Fix the bug that Richard pointed out (to do with hetero-kinded
        coercions)
      
      Then compiler performance worsened, which led mt do discover
      two performance bugs:
      
      * The smart constructor Coercion.mkNthCo didn't have a case
        for ForAllCos, which meant we stupidly build a complicated
        coercion where a simple one would do
      
      * In OptCoercion there was one place where we used CoherenceCo
        (the data constructor) rather than mkCoherenceCo (the smart
        constructor), which meant that the the stupid complicated
        coercion wasn't optimised away
      
      For reasons I don't fully understand, T5321Fun did 2% less compiler
      allocation after all this, which is good.
      b4f2afe7