1. 30 Sep, 2016 2 commits
    • Simon Peyton Jones's avatar
      Fix impredicativity (again) · b612da66
      Simon Peyton Jones authored
      This patch fixes Trac #12616.
      
      Dignosis.  In TcUnify.tc_sub_type_ds we were going to some trouble to
      support co- and contra-variance even for impredicative types.  With
      -XImpredicativeTYpes, this allowed a unification variable to be
      unified with a polytype (probably wrongly) and that caused later
      trouble in the constraint solver, where -XImpredicativeTypes was /not/
      on.  In effect, -XImpredicativeTypes can't be switched on locally.
      
      Why did we want ImpredicativeTypes locally?  Because the program
      generated by GND for a higher-rank method involved impredicative
      instantation of 'coerce':
            op = coerce op   -- where op has a higher rank type
      See Note [Newtype-deriving instances] in TcGenDeriv.
      
      Cure.
      
      1.  It is ghastly to rely on ImpredicativeTypes (a 100% flaky
          feature) to instantiate coerce polymorphically.  Happily we
          now have Visible Type Application, so I've used that instead
          which should be solid and reliable.
      
      2.  I deleted the code in tc_sub_type_ds that allows the constraint
          solver to "look through" a unification variable to find a
          polytype.  That used to be essential in the days of ReturnTv,
          but it's utterly unreliable and should be consigned to the dustbin
          of history.  (We have ExpType now for the essential uses.)
      
      Tests involving ImpredicativeTypes are affected, but I'm not worried
      about them... it's advertised as a feature you can't rely on, and
      I want to reform it outright.
      b612da66
    • Simon Peyton Jones's avatar
      Fix a bug in occurs checking · 66a8c194
      Simon Peyton Jones authored
      1. Trac #12593 exposed a long-standing bug in the occurs
         checking machinery.  When unifying two type variables
                a ~ b
         where a /= b, we were assuming that there could be
         no occurs-check error.  But there can: 'a' can occur
         in b's kind!  When the RHS was a non-tyvar we used
         occurCheckExpand, which /did/ look in kinds, but not
         when the RHS was a tyvar.
      
         This bug has been lurking ever since TypeInType, maybe
         longer.  And it was present both in TcUnify (the on-the-fly
         unifier), and in TcInteract.
      
         I ended up refactoring both so that the tyvar/tyvar
         path naturally goes through the same occurs-check as
         non-tyvar rhss.  It's simpler and more robust now.
      
         One good thing is that both unifiers now share
             TcType.swapOverVars
             TcType.canSolveByUnification
         previously they had different logic for the same goals
      
      2. Fixing this bug exposed another!  In T11635 we end
         up unifying
         (alpha :: forall k. k->*) ~ (beta :: forall k. k->*)
         Now that the occurs check is done for tyvars too, we
         look inside beta's kind.  And then reject the program
         becuase of the forall inside there.  But in fact that
         forall is fine -- it does not count as impredicative
         polymoprhism.   See Note [Checking for foralls]
         in TcType.
      
      3. All this fuss around occurrence checking forced me
         to look at TcUnify.checkTauTvUpdate
                and TcType.occurCheckExpand
         There's a lot of duplication there, and I managed
         to elminate quite a bit of it. For example,
         checkTauTvUpdate called a local 'defer_me'; and then
         called occurCheckExpand, which then used a very
         similar 'fast_check'.
      
         Things are better, but there is more to do.
      66a8c194
  2. 28 Sep, 2016 1 commit
  3. 12 Sep, 2016 3 commits
    • Simon Peyton Jones's avatar
      Testsuite wibbles, to the same files · ec3edd56
      Simon Peyton Jones authored
      Sigh.  I added some comments to the source files,
      and failed to revalidate.  But the comments change the
      line number in the error messages.  Doh.
      ec3edd56
    • Simon Peyton Jones's avatar
      Test wibbles for commit 03541cba · 5eeabe25
      Simon Peyton Jones authored
      I must have failed to validate properly, sorry.
      These testsuite wibbles belong with
      
        commit 03541cba
        Author: Simon Peyton Jones <simonpj@microsoft.com>
        Date:   Fri Sep 9 17:42:42 2016 +0100
      
            Be less picky about reporing inaccessible code
      5eeabe25
    • Simon Peyton Jones's avatar
      Be less picky about reporing inaccessible code · 03541cba
      Simon Peyton Jones authored
      Triggered by the discussion on Trac #12466, this patch
      makes GHC less aggressive about reporting an error when
      there are insoluble Givens.
      
      Being so agressive was making some libraries fail to
      compile, and is arguably wrong in at least some cases.
      See the discussion on the ticket.
      
      Several test now pass when they failed before; see
      the files-modified list for this patch.
      03541cba
  4. 31 Aug, 2016 1 commit
  5. 21 Aug, 2016 2 commits
  6. 17 Aug, 2016 1 commit
  7. 01 Aug, 2016 1 commit
  8. 20 Jul, 2016 1 commit
    • Ben Gamari's avatar
      InstEnv: Ensure that instance visibility check is lazy · ed480981
      Ben Gamari authored
      Previously instIsVisible had completely broken the laziness of
      lookupInstEnv' since it would examine is_dfun_name to check the name of
      the defining module (to know whether it is an interactive module). This
      resulted in the visibility check drawing in an interface file
      unnecessarily. This contributed to the unnecessary regression in
      compiler allocations reported in #12367.
      
      Test Plan: Validate, check nofib changes
      
      Reviewers: simonpj, ezyang, austin
      
      Reviewed By: ezyang
      
      Subscribers: thomie, ezyang
      
      Differential Revision: https://phabricator.haskell.org/D2411
      
      GHC Trac Issues: #12367
      ed480981
  9. 14 Jul, 2016 1 commit
  10. 05 Jul, 2016 1 commit
    • Simon Peyton Jones's avatar
      Check generic-default method for ambiguity · 85aa6ef0
      Simon Peyton Jones authored
      Fixes Trac #7497 and #12151.   In some earlier upheaval I introduced
      a bug in the ambiguity check for genreric-default method.
      
      This patch fixes it.  But in fixing it I realised that the
      sourc-location of any such error message was bogus, so I fixed
      that too, which involved a slightly wider change; see the
      comments with TcMethInfo.
      85aa6ef0
  11. 23 Jun, 2016 1 commit
  12. 22 Jun, 2016 2 commits
    • Simon Peyton Jones's avatar
      Improve error message in deriving( Functor ) · cc92a446
      Simon Peyton Jones authored
      Fixes Trac #12163.  Pretty simple.
      cc92a446
    • niteria's avatar
      Make the Ord Module independent of Unique order (2nd try) · 348f2dbb
      niteria authored
      The `Ord Module` instance currently uses `Unique`s for comparison.
      We don't want to use the `Unique` order because it can introduce
      nondeterminism.
      This switches `Ord ModuleName` and `Ord UnitId` to use lexicographic
      ordering making `Ord Module` deterministic transitively.
      
      I've run `nofib` and it doesn't make a measurable difference.
      
      See also Note [ModuleEnv determinism and performance].
      
      This fixes #12191 - the regression, that the previous version of this
      patch had.
      
      Test Plan:
      ./validate
      run nofib: P112
      
      Reviewers: simonmar, bgamari, austin
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2354
      
      GHC Trac Issues: #4012, #12191
      348f2dbb
  13. 20 Jun, 2016 2 commits
  14. 18 Jun, 2016 1 commit
    • Ryan Scott's avatar
      Add Bifoldable and Bitraversable to base · 270d545d
      Ryan Scott authored
      This adds `Data.Bifoldable` and `Data.Bitraversable` from the
      `bifunctors` package to `base`, completing the migration started in
      D336.  This is fairly straightforward, although there were a suprising
      amount of reinternal organization in `base` that was needed for this to
      happen:
      
      * `Data.Foldable`, `Data.Traversable`, `Data.Bifoldable`, and
        `Data.Bitraversable` share some nonexported datatypes (e.g., `StateL`,
        `StateR`, `Min`, `Max`, etc.) to implement some instances. To avoid
        code duplication, I migrated this internal code to a new hidden
        module, `Data.Functor.Utils` (better naming suggestions welcome).
      
      * `Data.Traversable` and `Data.Bitraversable` also make use of an
        identity newtype, so I modified them to use
        `Data.Functor.Identity.Identity`. This has a ripple effect on several
        other modules, since I had to move instances around in order to avoid
        dependency cycles.
      
      Fixes #10448.
      
      Reviewers: ekmett, hvr, austin, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2284
      
      GHC Trac Issues: #9682, #10448
      270d545d
  15. 15 Jun, 2016 3 commits
    • Simon Peyton Jones's avatar
      Major patch to introduce TyConBinder · e368f326
      Simon Peyton Jones authored
      Before this patch, following the TypeInType innovations,
      each TyCon had two lists:
        - tyConBinders :: [TyBinder]
        - tyConTyVars  :: [TyVar]
      
      They were in 1-1 correspondence and contained
      overlapping information.  More broadly, there were many
      places where we had to pass around this pair of lists,
      instead of a single list.
      
      This commit tidies all that up, by having just one list of
      binders in a TyCon:
      
        - tyConBinders :: [TyConBinder]
      
      The new data types look like this:
      
        Var.hs:
           data TyVarBndr tyvar vis = TvBndr tyvar vis
           data VisibilityFlag = Visible | Specified | Invisible
           type TyVarBinder = TyVarBndr TyVar VisibilityFlag
      
        TyCon.hs:
           type TyConBinder = TyVarBndr TyVar TyConBndrVis
      
           data TyConBndrVis
             = NamedTCB VisibilityFlag
             | AnonTCB
      
        TyCoRep.hs:
           data TyBinder
             = Named TyVarBinder
             | Anon Type
      
      Note that Var.TyVarBdr has moved from TyCoRep and has been
      made polymorphic in the tyvar and visiblity fields:
      
           type TyVarBinder = TyVarBndr TyVar VisibilityFlag
              -- Used in ForAllTy
           type TyConBinder = TyVarBndr TyVar TyConBndrVis
              -- Used in TyCon
      
           type IfaceForAllBndr  = TyVarBndr IfaceTvBndr VisibilityFlag
           type IfaceTyConBinder = TyVarBndr IfaceTvBndr TyConBndrVis
               -- Ditto, in interface files
      
      There are a zillion knock-on changes, but everything
      arises from these types.  It was a bit fiddly to get the
      module loops to work out right!
      
      Some smaller points
      ~~~~~~~~~~~~~~~~~~~
      * Nice new functions
          TysPrim.mkTemplateKiTyVars
          TysPrim.mkTemplateTyConBinders
        which help you make the tyvar binders for dependently-typed
        TyCons.  See comments with their definition.
      
      * The change showed up a bug in TcGenGenerics.tc_mkRepTy, where the code
        was making an assumption about the order of the kind variables in the
        kind of GHC.Generics.(:.:).  I fixed this; see TcGenGenerics.mkComp.
      e368f326
    • Simon Peyton Jones's avatar
      Re-add FunTy (big patch) · 77bb0927
      Simon Peyton Jones authored
      With TypeInType Richard combined ForAllTy and FunTy, but that was often
      awkward, and yielded little benefit becuase in practice the two were
      always treated separately.  This patch re-introduces FunTy.  Specfically
      
      * New type
          data TyVarBinder = TvBndr TyVar VisibilityFlag
        This /always/ has a TyVar it.  In many places that's just what
        what we want, so there are /lots/ of TyBinder -> TyVarBinder changes
      
      * TyBinder still exists:
          data TyBinder = Named TyVarBinder | Anon Type
      
      * data Type = ForAllTy TyVarBinder Type
                  | FunTy Type Type
                  |  ....
      
      There are a LOT of knock-on changes, but they are all routine.
      
      The Haddock submodule needs to be updated too
      77bb0927
    • Simon Peyton Jones's avatar
      Revert "Make the Ord Module independent of Unique order" · 70a45893
      Simon Peyton Jones authored
      This reverts commit 0497ee50.
      
      Reason: See Trac #12191.  I'm reverting pending Bartosz's
      investigation of what went wrong.
      70a45893
  16. 13 Jun, 2016 2 commits
    • niteria's avatar
      Make the Ord Module independent of Unique order · 0497ee50
      niteria authored
      The `Ord Module` instance currently uses `Unique`s for comparison.
      We don't want to use the `Unique` order because it can introduce nondeterminism.
      This switches `Ord ModuleName` and `Ord UnitId` to use lexicographic ordering
      making `Ord Module` deterministic transitively.
      
      I've run `nofib` and it doesn't make a measurable difference.
      
      See also Note [ModuleEnv determinism and performance].
      
      Test Plan:
      ./validate
      run nofib: P112
      
      Reviewers: simonpj, simonmar, austin, bgamari
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2030
      
      GHC Trac Issues: #4012
      0497ee50
    • Simon Peyton Jones's avatar
      Improve typechecking of let-bindings · 15b9bf4b
      Simon Peyton Jones authored
      This major commit was initially triggered by #11339, but it spiraled
      into a major review of the way in which type signatures for bindings
      are handled, especially partial type signatures.  On the way I fixed a
      number of other bugs, namely
         #12069
         #12033
         #11700
         #11339
         #11670
      
      The main change is that I completely reorganised the way in which type
      signatures in bindings are handled. The new story is in TcSigs
      Note [Overview of type signatures].  Some specific:
      
      * Changes in the data types for signatures in TcRnTypes:
        TcIdSigInfo and new TcIdSigInst
      
      * New module TcSigs deals with typechecking type signatures
        and pragmas. It contains code mostly moved from TcBinds,
        which is already too big
      
      * HsTypes: I swapped the nesting of HsWildCardBndrs
        and HsImplicitBndsrs, so that the wildcards are on the
        oustide not the insidde in a LHsSigWcType.  This is just
        a matter of convenient, nothing deep.
      
      There are a host of other changes as knock-on effects, and
      it all took FAR longer than I anticipated :-).  But it is
      a significant improvement, I think.
      
      Lots of error messages changed slightly, some just variants but
      some modest improvements.
      
      New tests
      
      * typecheck/should_compile
          * SigTyVars: a scoped-tyvar test
          * ExPat, ExPatFail: existential pattern bindings
          * T12069
          * T11700
          * T11339
      
      * partial-sigs/should_compile
          * T12033
          * T11339a
          * T11670
      
      One thing to check:
      
      * Small change to output from ghc-api/landmines.
        Need to check with Alan Zimmerman
      15b9bf4b
  17. 18 May, 2016 3 commits
  18. 10 May, 2016 2 commits
  19. 04 May, 2016 2 commits
    • niteria's avatar
      Kill non-deterministic foldUFM in TrieMap and TcAppMap · ad4392c1
      niteria authored
      Summary:
      foldUFM introduces unnecessary non-determinism that actually
      leads to different generated code as explained in
      Note [TrieMap determinism].
      
      As we're switching from UniqFM to UniqDFM here you might be
      concerned about performance. There's nothing that ./validate
      detects. nofib reports no change in Compile Allocations, but
      Compile Time got better on some tests and worse on some,
      yielding this summary:
      
              -1 s.d.                -----            -3.8%
              +1 s.d.                -----            +5.4%
              Average                -----            +0.7%
      
      This is not a fair comparison as the order of Uniques
      changes what GHC is actually doing. One benefit from making
      this deterministic is also that it will make the
      performance results more stable.
      
      Full nofib results: P108
      
      Test Plan: ./validate, nofib
      
      Reviewers: goldfire, simonpj, simonmar, austin, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2169
      
      GHC Trac Issues: #4012
      ad4392c1
    • Iavor S. Diatchki's avatar
      Be more aggressive when checking constraints for custom type errors. · b75d1940
      Iavor S. Diatchki authored
      This fixes #11990.
      
      The current rule is simpler than before: if we encounter an unsolved
      constraint that contains any mentions of properly applied `TypeError`,
      then we report the type error.
      
      If there are multiple `TypeErrors`, then we just report one of them.
      
      Reviewers: simonpj, bgamari, austin
      
      Reviewed By: simonpj
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2151
      
      GHC Trac Issues: #11990
      b75d1940
  20. 28 Apr, 2016 1 commit
  21. 26 Apr, 2016 2 commits
    • niteria's avatar
      Kill varSetElems in TcErrors · 2dc5b92e
      niteria authored
      The uses of varSetElems in these places are unnecessary and while it
      doesn't intruduce non-determinism in the ABI the plan is to get
      rid of all varSetElems to get some compile time guarantees.
      
      Test Plan: ./validate
      
      Reviewers: austin, simonmar, bgamari, goldfire, simonpj
      
      Reviewed By: simonpj
      
      Subscribers: thomie
      
      Differential Revision: https://phabricator.haskell.org/D2141
      
      GHC Trac Issues: #4012
      2dc5b92e
    • niteria's avatar
      Kill varSetElemsWellScoped in quantifyTyVars · c9bcaf31
      niteria authored
      varSetElemsWellScoped introduces unnecessary non-determinism in
      inferred type signatures.
      Removing this instance required changing the representation of
      TcDepVars to use deterministic sets.
      This is the last occurence of varSetElemsWellScoped, allowing me to
      finally remove it.
      
      Test Plan:
      ./validate
      I will update the expected outputs when commiting, some reordering
      of type variables in types is expected.
      
      Reviewers: goldfire, simonpj, austin, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: thomie, simonmar
      
      Differential Revision: https://phabricator.haskell.org/D2135
      
      GHC Trac Issues: #4012
      c9bcaf31
  22. 22 Apr, 2016 2 commits
    • Simon Peyton Jones's avatar
      Warn about simplifiable class constraints · 9421b0c7
      Simon Peyton Jones authored
      Provoked by Trac #11948, this patch adds a new warning to GHC
      
        -Wsimplifiable-class-constraints
      
      It warns if you write a class constraint in a type signature that
      can be simplified by an existing instance declaration.  Almost always
      this means you should simplify it right now; type inference is very
      fragile without it, as #11948 shows.
      
      I've put the warning as on-by-default, but I suppose that if there are
      howls of protest we can move it out (as happened for -Wredundant-constraints.
      
      It actually found an example of an over-complicated context in CmmNode.
      
      Quite a few tests use these weird contexts to trigger something else,
      so I had to suppress the warning in those.
      
      The 'haskeline' library has a few occurrences of the warning (which
      I think should be fixed), so I switched it off for that library in
      warnings.mk.
      
      The warning itself is done in TcValidity.check_class_pred.
      
      HOWEVER, when type inference fails we get a type error; and the error
      suppresses the (informative) warning.  So as things stand, the warning
      only happens when it doesn't cause a problem.  Not sure what to do
      about this, but this patch takes us forward, I think.
      9421b0c7
    • Simon Peyton Jones's avatar
      Do not use defaulting in ambiguity check · edf54d72
      Simon Peyton Jones authored
      This fixes Trac #11947.  See TcSimplify
      Note [No defaulting in the ambiguity check]
      edf54d72
  23. 20 Apr, 2016 1 commit
    • Simon Peyton Jones's avatar
      SCC analysis for instances as well as types/classes · 353d8ae6
      Simon Peyton Jones authored
      This big patch is in pursuit of Trac #11348.
      
      It is largely the work of Alex Veith (thank you!), with some
      follow-up simplification and refactoring from Simon PJ.
      
      The main payload is described in RnSource
        Note [Dependency analysis of type, class, and instance decls]
      which is pretty detailed.
      
      * There is a new data type HsDecls.TyClGroup, for a strongly
        connected component of type/class/instance/role decls.
      
        The hs_instds field of HsGroup disappears, in consequence
      
        This forces some knock-on changes, including a minor
        haddock submodule update
      
      Smaller, weakly-related things
      
      * I found that both the renamer and typechecker were building an
        identical env for RoleAnnots, so I put common code for
        RoleAnnotEnv in RnEnv.
      
      * I found that tcInstDecls1 had very clumsy error handling, so I
        put it together into TcInstDcls.doClsInstErrorChecks
      353d8ae6
  24. 19 Apr, 2016 2 commits
    • Simon Peyton Jones's avatar
      Tighten checking for associated type instances · 8136a5cb
      Simon Peyton Jones authored
      This patch finishes off Trac #11450.  Following debate on that ticket,
      the patch tightens up the rules for what the instances of an
      associated type can look like.  Now they must match the instance
      header exactly. Eg
      
         class C a b where
          type T a x b
      
      With this class decl, if we have an instance decl
        instance C ty1 ty2 where ...
      then the type instance must look like
           type T ty1 v ty2 = ...
      with exactly
        - 'ty1' for 'a'
        -  'ty2' for 'b', and
        - a variable for 'x'
      
      For example:
      
        instance C [p] Int
          type T [p] y Int = (p,y,y)
      
      Previously we allowed multiple instance equations and now, in effect,
      we don't since they would all overlap.  If you want multiple cases,
      use an auxiliary type family.
      
      This is consistent with the treatment of generic-default instances,
      and the user manual always said "WARNING: this facility (multiple
      instance equations may be withdrawn in the future".
      
      I also improved error messages, and did other minor refactoring.
      8136a5cb
    • Simon Peyton Jones's avatar
      Define TyCoRep.ppSuggestExplicitKinds, and use it · d59939a4
      Simon Peyton Jones authored
      This just defines a useful helper function that was being
      duplicated in several places
      d59939a4