1. 26 Mar, 2018 1 commit
    • Ryan Scott's avatar
      Fix two pernicious bugs in DeriveAnyClass · d8dbe293
      Ryan Scott authored
      The way GHC was handling `DeriveAnyClass` was subtly wrong
      in two notable ways:
      
      * In `inferConstraintsDAC`, we were //always// bumping the `TcLevel`
        of newly created unification variables, under the assumption that
        we would always place those unification variables inside an
        implication constraint. But #14932 showed precisely the scenario
        where we had `DeriveAnyClass` //without// any of the generated
        constraints being used inside an implication, which made GHC
        incorrectly believe the unification variables were untouchable.
      * Even worse, we were using the generated unification variables from
        `inferConstraintsDAC` in every single iteration of `simplifyDeriv`.
        In #14933, however, we have a scenario where we fill in a
        unification variable with a skolem in one iteration, discard it,
        proceed on to another iteration, use the same unification variable
        (still filled in with the old skolem), and try to unify it with
        a //new// skolem! This results in an utter disaster.
      
      The root of both these problems is `inferConstraintsDAC`. This patch
      fixes the issue by no longer generating unification variables
      directly in `inferConstraintsDAC`. Instead, we store the original
      variables from a generic default type signature in `to_metas`, a new
      field of `ThetaOrigin`, and in each iteration of `simplifyDeriv`, we
      generate fresh meta tyvars (avoiding the second issue). Moreover,
      this allows us to more carefully fine-tune the `TcLevel` under which
      we create these meta tyvars, fixing the first issue.
      
      Test Plan: make test TEST="T14932 T14933"
      
      Reviewers: simonpj, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14932, #14933
      
      Differential Revision: https://phabricator.haskell.org/D4507
      
      (cherry picked from commit 98930426)
      d8dbe293
  2. 08 Nov, 2017 1 commit
    • Simon Peyton Jones's avatar
      Minimise provided dictionaries in pattern synonyms · 2c2f3cea
      Simon Peyton Jones authored
      Trac #14394 showed that it's possible to get redundant
      constraints in the inferred provided constraints of a pattern
      synonym.  This patch removes the redundancy with mkMinimalBySCs.
      
      To do this I had to generalise the type of mkMinimalBySCs slightly.
      And, to reduce confusing reversal, I made it stable: it now returns
      its result in the same order as its input.  That led to a raft of
      error message wibbles, mostly for the better.
      2c2f3cea
  3. 11 Oct, 2017 1 commit
  4. 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
  5. 16 Aug, 2017 1 commit
  6. 15 Aug, 2017 1 commit
    • Ryan Scott's avatar
      Use a ReaderT in TcDeriv to avoid some tedious plumbing · ed7a830d
      Ryan Scott authored
      Addresses point (2) of https://phabricator.haskell.org/D3337#107865.
      
      Before, several functions in `TcDeriv` and `TcDerivInfer` which compute
      an `EarlyDerivSpec` were manually threading through about 10 different
      arguments, which contribute to quite a lot of clutter whenever they need
      to be updated. To minimize this plumbing, and to make it clearer which
      of these 10 values are being used where, I refactored the code in
      `TcDeriv` and `TcDerivInfer` to use a new `DerivM` type:
      
      ```lang=haskell
      type DerivM = ReaderT DerivEnv TcRn
      ```
      
      where `DerivEnv` contains the 10 aforementioned values. In addition to
      cleaning up the code, this should make some subsequent changes planned
      for later less noisy.
      
      Test Plan: ./validate
      
      Reviewers: austin, bgamari
      
      Subscribers: rwbarton, thomie
      
      Differential Revision: https://phabricator.haskell.org/D3846
      ed7a830d
  7. 12 Aug, 2017 1 commit
    • Ryan Scott's avatar
      Split out inferConstraintsDataConArgs from inferConstraints · a4f347c2
      Ryan Scott authored
      Summary:
      Addresses point (1) of https://phabricator.haskell.org/D3337#107865.
      
      Before, `inferConstraints` awkwardly combined all of the logic needed to handle
      stock, newtype, and anyclass deriving. Really, though, the stock/newtype logic
      is quite different from the anyclass logic, so this splits off
      `inferConstraintsDataConArgs` (so named because it infers constraints by
      inspecting the types of the arguments to data constructors) from
      `inferConstraints` to handle the stock/newtype-specific bits.
      
      Aside from making the code somewhat clearer, this allows us to factor out
      superclass constraint inference, which is done regardless of deriving strategy.
      
      Test Plan: If it builds, ship it
      
      Reviewers: bgamari, austin
      
      Subscribers: rwbarton, thomie
      
      Differential Revision: https://phabricator.haskell.org/D3827
      a4f347c2
  8. 26 Jul, 2017 1 commit
  9. 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
  10. 10 Mar, 2017 1 commit
  11. 06 Mar, 2017 1 commit
  12. 21 Feb, 2017 4 commits
    • Ryan Scott's avatar
      Replace some pushTcLevelM's with pushTcLevelM_ · 611f998f
      Ryan Scott authored
      These occurrences of pushTcLevelM weren't using the resulting TcLevel,
      so they can be replaced with the (ostensibly more efficient) pushTcLevelM_.
      
      No change in behavior.
      611f998f
    • Ryan Scott's avatar
      Minor spelling, grammar, and formatting fixes · 4080a63c
      Ryan Scott authored
      [ci skip]
      4080a63c
    • Simon Peyton Jones's avatar
      Refactor inferConstraints not to use CPS · 95cbb55c
      Simon Peyton Jones authored
      For some odd reason inferConstraints was using a CPS style,
      which is entirely unnecessary.  This patch straightens it out.
      
      No change in what it does.
      95cbb55c
    • Simon Peyton Jones's avatar
      Fix DeriveAnyClass (again) · fd841f87
      Simon Peyton Jones authored
      This patch fixes Trac #13272.  The general approach was fine, but
      we were simply not generating the correct implication constraint
      (in particular generating fresh unification variables).  I added
      a lot more commentary to Note [Gathering and simplifying
      constraints for DeriveAnyClass]
      
      I'm still not very happy with the overall architecture.  It feels
      more complicate than it should.
      fd841f87
  13. 10 Feb, 2017 1 commit
    • Ryan Scott's avatar
      Refactor DeriveAnyClass's instance context inference · 639e702b
      Ryan Scott authored
      Summary:
      Currently, `DeriveAnyClass` has two glaring flaws:
      
      * It only works on classes whose argument is of kind `*` or `* -> *` (#9821).
      * The way it infers constraints makes no sense. It basically co-opts the
        algorithms used to infer contexts for `Eq` (for `*`-kinded arguments) or
        `Functor` (for `(* -> *)`-kinded arguments). This tends to produce overly
        constrained instances, which in extreme cases can lead to legitimate things
        failing to typecheck (#12594). Or even worse, it can trigger GHC panics
        (#12144 and #12423).
      
      This completely reworks the way `DeriveAnyClass` infers constraints to fix
      these two issues. It now uses the type signatures of the derived class's
      methods to infer constraints (and to simplify them). A high-level description
      of how this works is included in the GHC users' guide, and more technical notes
      on what is going on can be found as comments (and a Note) in `TcDerivInfer`.
      
      Fixes #9821, #12144, #12423, #12594.
      
      Test Plan: ./validate
      
      Reviewers: dfeuer, goldfire, simonpj, austin, bgamari
      
      Subscribers: dfeuer, thomie
      
      Differential Revision: https://phabricator.haskell.org/D2961
      639e702b
  14. 13 Nov, 2016 1 commit
    • Ben Gamari's avatar
      Kill Type pretty-printer · 6c0f10fa
      Ben Gamari authored
      Here we consolidate the pretty-printing logic for types in IfaceType. We
      need IfaceType regardless and the printer for Type can be implemented in
      terms of that for IfaceType. See #11660.
      
      Note that this is very much a work-in-progress. Namely I still have yet
      to ponder how to ease the hs-boot file situation, still need to rip out
      more dead code, need to move some of the special cases for, e.g., `*` to
      the IfaceType printer, and need to get it to validate. That being said,
      it comes close to validating as-is.
      
      Test Plan: Validate
      
      Reviewers: goldfire, austin
      
      Subscribers: goldfire, thomie, simonpj
      
      Differential Revision: https://phabricator.haskell.org/D2528
      
      GHC Trac Issues: #11660
      6c0f10fa
  15. 06 Oct, 2016 1 commit
    • Ryan Scott's avatar
      Refactor TcDeriv and TcGenDeriv · 4a03012a
      Ryan Scott authored
      Summary:
      Keeping a promise I made to Simon to clean up these modules.
      
      This change splits up the massive `TcDeriv` and `TcGenDeriv` modules into
      somewhat more manageable pieces. The new modules are:
      
      * `TcGenFunctor`: This contains the deriving machinery for `Functor`,
        `Foldable`, and `Traversable` (which all use the same underlying algorithm).
      * `TcDerivInfer`: This is the new home for `inferConstraints`,
        `simplifyInstanceContexts`, and related functions, whose role is to come up
        with the derived instance context and subsequently simplify it.
      * `TcDerivUtils`: This is a grab-bag module that contains several
        error-checking utilities originally in `TcDeriv`, as well as some functions
        that `TcDeriv` and `TcDerivInfer` both need.
      
      The end result is that `TcDeriv` is now less than 1,600 SLOC (originally 2,686
      SLOC), and `TcGenDeriv` is now about 2,000 SLOC (originally 2,964).
      
      In addition, this also implements a couple of tiny refactorings:
      
      * I transformed `type Condition = (DynFlags, TyCon) -> Validity` into
        `type Condition = DynFlags -> TyCon -> Validity`
      * I killed the `DerivSpecGeneric` constructor for `DerivSpecMechanism`, and
        merged its functionality into `DerivSpecStock`. In addition,
        `hasStockDeriving` now contains key-value pairs for `Generic` and `Generic1`,
        so they're no longer treated as an awkward special case in `TcDeriv`.
      
      Test Plan: ./validate
      
      Reviewers: simonpj, austin, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: thomie, mpickering
      
      Differential Revision: https://phabricator.haskell.org/D2568
      4a03012a