Skip to content
Snippets Groups Projects
  1. Jul 26, 2019
    • Ryan Scott's avatar
      Banish reportFloatingViaTvs to the shadow realm (#15831, #16181) · 30b6f391
      Ryan Scott authored and Marge Bot's avatar Marge Bot committed
      GHC used to reject programs of this form:
      
      ```
      newtype Age = MkAge Int
        deriving Eq via Const Int a
      ```
      
      That's because an earlier implementation of `DerivingVia` would
      generate the following instance:
      
      ```
      instance Eq Age where
        (==) = coerce @(Const Int a -> Const Int a -> Bool)
                      @(Age         -> Age         -> Bool)
                      (==)
      ```
      
      Note that the `a` in `Const Int a` is not bound anywhere, which
      causes all sorts of issues. I figured that no one would ever want to
      write code like this anyway, so I simply banned "floating" `via` type
      variables like `a`, checking for their presence in the aptly named
      `reportFloatingViaTvs` function.
      
      `reportFloatingViaTvs` ended up being implemented in a subtly
      incorrect way, as #15831 demonstrates. Following counsel with the
      sage of gold fire, I decided to abandon `reportFloatingViaTvs`
      entirely and opt for a different approach that would _accept_
      the instance above. This is because GHC now generates this instance
      instead:
      
      ```
      instance forall a. Eq Age where
        (==) = coerce @(Const Int a -> Const Int a -> Bool)
                      @(Age         -> Age         -> Bool)
                      (==)
      ```
      
      Notice that we now explicitly quantify the `a` in
      `instance forall a. Eq Age`, so everything is peachy scoping-wise.
      See `Note [Floating `via` type variables]` in `TcDeriv` for the full
      scoop.
      
      A pleasant benefit of this refactoring is that it made it much easier
      to catch the problem observed in #16181, so this patch fixes that
      issue too.
      
      Fixes #15831. Fixes #16181.
      30b6f391
  2. Jul 19, 2019
  3. Jul 11, 2019
    • Ryan Scott's avatar
      Don't typecheck too much (or too little) in DerivingVia (#16923) · b507aceb
      Ryan Scott authored and Marge Bot's avatar Marge Bot committed
      Previously, GHC would typecheck the `via` type once per class in a
      `deriving` clause, which caused the problems observed in #16923.
      This patch restructures some of the functionality in `TcDeriv` and
      `TcHsType` to avoid this problem. We now typecheck the `via` type
      exactly once per `deriving` clause and *then* typecheck all of the
      classes in the clause.
      See `Note [Don't typecheck too much in DerivingVia]` in `TcDeriv`
      for the full details.
      b507aceb
  4. Jul 05, 2019
    • Vladislav Zavialov's avatar
      Produce all DerivInfo in tcTyAndClassDecls · 679427f8
      Vladislav Zavialov authored and Marge Bot's avatar Marge Bot committed
      Before this refactoring:
      
      * DerivInfo for data family instances was returned from tcTyAndClassDecls
      * DerivInfo for data declarations was generated with mkDerivInfos and added at a
        later stage of the pipeline in tcInstDeclsDeriv
      
      After this refactoring:
      
      * DerivInfo for both data family instances and data declarations is returned from
        tcTyAndClassDecls in a single list.
      
      This uniform treatment results in a more convenient arrangement to fix #16731.
      679427f8
  5. May 24, 2019
    • Ryan Scott's avatar
      Some forall-related cleanup in deriving code · 6eedbd83
      Ryan Scott authored and Marge Bot's avatar Marge Bot committed
      * Tweak the parser to allow `deriving` clauses to mention explicit
        `forall`s or kind signatures without gratuitous parentheses.
        (This fixes #14332 as a consequence.)
      * Allow Haddock comments on `deriving` clauses with explicit
        `forall`s. This requires corresponding changes in Haddock.
      6eedbd83
  6. Apr 19, 2019
    • Alec Theriault's avatar
      TH: make `Lift` and `TExp` levity-polymorphic · 57cf1133
      Alec Theriault authored and Marge Bot's avatar Marge Bot committed
      Besides the obvious benefits of being able to manipulate `TExp`'s of
      unboxed types, this also simplified `-XDeriveLift` all while making
      it more capable.
      
        * `ghc-prim` is explicitly depended upon by `template-haskell`
      
        * The following TH things are parametrized over `RuntimeRep`:
      
            - `TExp(..)`
            - `unTypeQ`
            - `unsafeTExpCoerce`
            - `Lift(..)`
      
        * The following instances have been added to `Lift`:
      
            - `Int#`, `Word#`, `Float#`, `Double#`, `Char#`, `Addr#`
            - unboxed tuples of lifted types up to arity 7
            - unboxed sums of lifted types up to arity 7
      
          Ideally we would have levity-polymorphic _instances_ of unboxed
          tuples and sums.
      
        * The code generated by `-XDeriveLift` uses expression quotes
          instead of generating large amounts of TH code and having
          special hard-coded cases for some unboxed types.
      57cf1133
  7. Apr 04, 2019
    • Ryan Scott's avatar
      Fix #16518 with some more kind-splitting smarts · 25c02ea1
      Ryan Scott authored and Marge Bot's avatar Marge Bot committed
      This patch corrects two simple oversights that led to #16518:
      
      1. `HsUtils.typeToLHsType` was taking visibility into account in the
         `TyConApp` case, but not the `AppTy` case. I've factored out the
         visibility-related logic into its own `go_app` function and now
         invoke `go_app` from both the `TyConApp` and `AppTy` cases.
      2. `Type.fun_kind_arg_flags` did not properly split kinds with
         nested `forall`s, such as
         `(forall k. k -> Type) -> (forall k. k -> Type)`. This was simply
         because `fun_kind_arg_flags`'s `FunTy` case always bailed out and
         assumed all subsequent arguments were `Required`, which clearly
         isn't the case for nested `forall`s. I tweaked the `FunTy` case
         to recur on the result kind.
      25c02ea1
  8. Mar 25, 2019
    • Takenobu Tani's avatar
      Update Wiki URLs to point to GitLab · 3769e3a8
      Takenobu Tani authored and Marge Bot's avatar Marge Bot committed
      This moves all URL references to Trac Wiki to their corresponding
      GitLab counterparts.
      
      This substitution is classified as follows:
      
      1. Automated substitution using sed with Ben's mapping rule [1]
          Old: ghc.haskell.org/trac/ghc/wiki/XxxYyy...
          New: gitlab.haskell.org/ghc/ghc/wikis/xxx-yyy...
      
      2. Manual substitution for URLs containing `#` index
          Old: ghc.haskell.org/trac/ghc/wiki/XxxYyy...#Zzz
          New: gitlab.haskell.org/ghc/ghc/wikis/xxx-yyy...#zzz
      
      3. Manual substitution for strings starting with `Commentary`
          Old: Commentary/XxxYyy...
          New: commentary/xxx-yyy...
      
      See also !539
      
      [1]: https://gitlab.haskell.org/bgamari/gitlab-migration/blob/master/wiki-mapping.json
      3769e3a8
  9. Mar 15, 2019
  10. Feb 14, 2019
    • Alec Theriault's avatar
      Add `liftedTyped` to `Lift` class · 7f26b74e
      Alec Theriault authored and Marge Bot's avatar Marge Bot committed
      Implements GHC proposal 43, adding a `liftTyped` method to the `Lift` typeclass.
      This also adds some documentation to `TExp`, describing typed splices and their
      advantages over their untyped counterparts.
      
      Resolves #14671.
      7f26b74e
  11. Feb 06, 2019
  12. Jan 30, 2019
  13. Jan 20, 2019
  14. Jan 03, 2019
    • My Nguyen's avatar
      Visible kind application · 17bd1635
      My Nguyen authored
      Summary:
      This patch implements visible kind application (GHC Proposal 15/#12045), as well as #15360 and #15362.
      It also refactors unnamed wildcard handling, and requires that type equations in type families in Template Haskell be
      written with full type on lhs. PartialTypeSignatures are on and warnings are off automatically with visible kind
      application, just like in term-level.
      
      There are a few remaining issues with this patch, as documented in
      ticket #16082.
      
      Includes a submodule update for Haddock.
      
      Test Plan: Tests T12045a/b/c/TH1/TH2, T15362, T15592a
      
      Reviewers: simonpj, goldfire, bgamari, alanz, RyanGlScott, Iceland_jack
      
      Subscribers: ningning, Iceland_jack, RyanGlScott, int-index, rwbarton, mpickering, carter
      
      GHC Trac Issues: `#12045`, `#15362`, `#15592`, `#15788`, `#15793`, `#15795`, `#15797`, `#15799`, `#15801`, `#15807`, `#15816`
      
      Differential Revision: https://phabricator.haskell.org/D5229
      17bd1635
  15. Nov 07, 2018
    • davide's avatar
      testsuite: Save performance metrics in git notes. · 932cd41d
      davide authored
      This patch makes the following improvement:
        - Automatically records test metrics (per test environment) so that
          the programmer need not supply nor update expected values in *.T
          files.
          - On expected metric changes, the programmer need only indicate the
            direction of change in the git commit message.
        - Provides a simple python tool "perf_notes.py" to compare metrics
          over time.
      
      Issues:
        - Using just the previous commit allows performance to drift with each
          commit.
          - Currently we allow drift as we have a preference for minimizing
            false positives.
          - Some possible alternatives include:
            - Use metrics from a fixed commit per test: the last commit that
              allowed a change in performance (else the oldest metric)
            - Or use some sort of aggregate since the last commit that allowed
              a change in performance (else all available metrics)
            - These alternatives may result in a performance issue (with the
              test driver) having to heavily search git commits/notes.
        - Run locally, performance tests will trivially pass unless the tests
          were run locally on the previous commit. This is often not the case
          e.g.  after pulling recent changes.
      
      Previously, *.T files contain statements such as:
      ```
      stats_num_field('peak_megabytes_allocated', (2, 1))
      compiler_stats_num_field('bytes allocated',
                               [(wordsize(64), 165890392, 10)])
      ```
      This required the programmer to give the expected values and a tolerance
      deviation (percentage). With this patch, the above statements are
      replaced with:
      ```
      collect_stats('peak_megabytes_allocated', 5)
      collect_compiler_stats('bytes allocated', 10)
      ```
      So that programmer must only enter which metrics to test and a tolerance
      deviation. No expected value is required. CircleCI will then run the
      tests per test environment and record the metrics to a git note for that
      commit and push them to the git.haskell.org ghc repo. Metrics will be
      compared to the previous commit. If they are different by the tolerance
      deviation from the *.T file, then the corresponding test will fail. By
      adding to the git commit message e.g.
      ```
       # Metric (In|De)crease <metric(s)> <options>: <tests>
      Metric Increase ['bytes allocated', 'peak_megabytes_allocated'] \
               (test_env='linux_x86', way='default'):
          Test012, Test345
      Metric Decrease 'bytes allocated':
          Test678
      Metric Increase:
          Test711
      ```
      This will allow the noted changes (letting the test pass). Note that by
      omitting metrics or options, the change will apply to all possible
      metrics/options (i.e. in the above, an increase for all metrics in all
      test environments is allowed for Test711)
      
      phabricator will use the message in the description
      
      Reviewers: bgamari, hvr
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, carter
      
      GHC Trac Issues: #12758
      
      Differential Revision: https://phabricator.haskell.org/D5059
      932cd41d
  16. Oct 15, 2018
    • Vladislav Zavialov's avatar
      Enable -Wcompat=error in the testsuite · 165d3d5d
      Vladislav Zavialov authored and Ben Gamari's avatar Ben Gamari committed
      Enabling -Werror=compat in the testsuite allows us to easily see the
      impact that a new warning has on code. It also means that in the period
      between adding the warning and making the actual breaking change, all
      new test cases that are being added to the testsuite will be
      forwards-compatible. This is good because it will make the actual
      breaking change contain less irrelevant testsuite updates.
      
      Things that -Wcompat warns about are things that are going to break in
      the future, so we can be proactive and keep our testsuite
      forwards-compatible.
      
      This patch consists of two main changes:
      
      * Add `TEST_HC_OPTS += -Werror=compat` to the testsuite configuration.
      * Fix all broken test cases.
      
      Test Plan: Validate
      
      Reviewers: hvr, goldfire, bgamari, simonpj, RyanGlScott
      
      Reviewed By: goldfire, RyanGlScott
      
      Subscribers: rwbarton, carter
      
      GHC Trac Issues: #15278
      
      Differential Revision: https://phabricator.haskell.org/D5200
      165d3d5d
  17. Oct 01, 2018
    • Ryan Scott's avatar
      Fix #15637 by using VTA more in GND · 309438e9
      Ryan Scott authored
      Summary:
      The code that GND was generating before could crumple over
      if it derived an instance for a class with an ambiguous type variable
      in the class head, such as the example in #15637. The solution is
      straightforward: simply instantiate all variables bound by the class
      head explicitly using visible type application, which will nip any
      ambiguity in the bud.
      
      Test Plan: make test TEST=T15637
      
      Reviewers: bgamari, simonpj, goldfire
      
      Reviewed By: simonpj
      
      Subscribers: simonpj, rwbarton, carter
      
      GHC Trac Issues: #15637
      
      Differential Revision: https://phabricator.haskell.org/D5148
      309438e9
  18. Jul 24, 2018
    • Ryan Scott's avatar
      Suppress -Winaccessible-code in derived code · 44a7b9ba
      Ryan Scott authored and Krzysztof Gogolewski's avatar Krzysztof Gogolewski committed
      Summary:
      It's rather unfortunate that derived code can produce inaccessible
      code warnings (as demonstrated in #8128, #8740, and #15398), since
      the programmer has no control over the generated code. This patch
      aims to suppress `-Winaccessible-code` in all derived code. It
      accomplishes this by doing the following:
      
      * Generalize the `ic_env :: TcLclEnv` field of `Implication` to
        be of type `Env TcGblEnc TcLclEnv` instead. This way, it also
        captures `DynFlags`, which record the flag state at the time
        the `Implication` was created.
      * When typechecking derived code, turn off `-Winaccessible-code`.
        This way, any insoluble given `Implication`s that are created when
        typechecking this derived code will remember that
        `-Winaccessible-code` was disabled.
      * During error reporting, consult the `DynFlags` of an
        `Implication` before making the decision to report an inaccessible
        code warning.
      
      Test Plan: make test TEST="T8128 T8740 T15398"
      
      Reviewers: simonpj, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: monoidal, rwbarton, thomie, carter
      
      GHC Trac Issues: #8128, #8740, #15398
      
      Differential Revision: https://phabricator.haskell.org/D4993
      44a7b9ba
  19. Jul 10, 2018
    • Simon Peyton Jones's avatar
      More refactoring in TcValidity · fd0f0334
      Simon Peyton Jones authored
      This patch responds to Trac #15334 by making it an error to
      write an instance declaration for a tuple constraint like
      (Eq [a], Show [a]).
      
      I then discovered that instance validity checking was
      scattered betweeen TcInstDcls and TcValidity, so I took
      the time to bring it all together, into
        TcValidity.checkValidInstHead
      
      In doing so I discovered that there are lot of special
      cases.   I have not changed them, but at least they are
      all laid out clearly now.
      fd0f0334
  20. Jul 06, 2018
  21. Jul 05, 2018
    • Ryan Scott's avatar
      Instantiate GND bindings with an explicit type signature · 132273f3
      Ryan Scott authored
      Summary:
      Before, we were using visible type application to apply
      impredicative types to `coerce` in
      `GeneralizedNewtypeDeriving`-generated bindings. This approach breaks
      down when combined with `QuantifiedConstraints` in certain ways,
      which #14883 and #15290 provide examples of. See
      Note [GND and QuantifiedConstraints] for all the gory details.
      
      To avoid this issue, we instead use an explicit type signature to
      instantiate each GND binding, and use that to bind any type variables
      that might be bound by a class method's type signature. This reduces
      the need to impredicative type applications, and more importantly,
      makes the programs from #14883 and #15290 work again.
      
      Test Plan: make test TEST="T15290b T15290c T15290d T14883"
      
      Reviewers: simonpj, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14883, #15290
      
      Differential Revision: https://phabricator.haskell.org/D4895
      132273f3
    • Ryan Scott's avatar
      Fix #15307 by making nlHsFunTy parenthesize more · 59a15a56
      Ryan Scott authored
      Summary:
      `nlHsFunTy` wasn't parenthesizing its arguments at all,
      which led to `-ddump-deriv` producing incorrectly parenthesized
      types (since it uses `nlHsFunTy` to construct those types), as
      demonstrated in #15307. Fix this by changing `nlHsFunTy` to add
      parentheses à la `ppr_ty`: always parenthesizing the argument type
      with function precedence, and recursively processing the result type,
      adding parentheses for each function type it encounters.
      
      Test Plan: make test TEST=T14578
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #15307
      
      Differential Revision: https://phabricator.haskell.org/D4890
      59a15a56
  22. Jun 15, 2018
  23. Jun 14, 2018
    • Vladislav Zavialov's avatar
      Embrace -XTypeInType, add -XStarIsType · d650729f
      Vladislav Zavialov authored
      Summary:
      Implement the "Embrace Type :: Type" GHC proposal,
      .../ghc-proposals/blob/master/proposals/0020-no-type-in-type.rst
      
      GHC 8.0 included a major change to GHC's type system: the Type :: Type
      axiom. Though casual users were protected from this by hiding its
      features behind the -XTypeInType extension, all programs written in GHC
      8+ have the axiom behind the scenes. In order to preserve backward
      compatibility, various legacy features were left unchanged. For example,
      with -XDataKinds but not -XTypeInType, GADTs could not be used in types.
      Now these restrictions are lifted and -XTypeInType becomes a redundant
      flag that will be eventually deprecated.
      
      * Incorporate the features currently in -XTypeInType into the
        -XPolyKinds and -XDataKinds extensions.
      * Introduce a new extension -XStarIsType to control how to parse * in
        code and whether to print it in error messages.
      
      Test Plan: Validate
      
      Reviewers: goldfire, hvr, bgamari, alanz, simonpj
      
      Reviewed By: goldfire, simonpj
      
      Subscribers: rwbarton, thomie, mpickering, carter
      
      GHC Trac Issues: #15195
      
      Differential Revision: https://phabricator.haskell.org/D4748
      d650729f
  24. Jun 05, 2018
    • Ryan Scott's avatar
      Introduce DerivingVia · 8ed8b037
      Ryan Scott authored
      This implements the `DerivingVia` proposal put forth in
      https://github.com/ghc-proposals/ghc-proposals/pull/120.
      
      This introduces the `DerivingVia` deriving strategy. This is a
      generalization of `GeneralizedNewtypeDeriving` that permits the user
      to specify the type to `coerce` from.
      
      The major change in this patch is the introduction of the
      `ViaStrategy` constructor to `DerivStrategy`, which takes a type
      as a field. As a result, `DerivStrategy` is no longer a simple
      enumeration type, but rather something that must be renamed and
      typechecked. The process by which this is done is explained more
      thoroughly in section 3 of this paper
      ( https://www.kosmikus.org/DerivingVia/deriving-via-paper.pdf ),
      although I have inlined the relevant parts into Notes where possible.
      
      There are some knock-on changes as well. I took the opportunity to
      do some refactoring of code in `TcDeriv`, especially the
      `mkNewTypeEqn` function, since it was bundling all of the logic for
      (1) deriving instances for newtypes and
      (2) `GeneralizedNewtypeDeriving`
      into one huge broth. `DerivingVia` reuses much of part (2), so that
      was factored out as much as possible.
      
      Bumps the Haddock submodule.
      
      Test Plan: ./validate
      
      Reviewers: simonpj, bgamari, goldfire, alanz
      
      Subscribers: alanz, goldfire, rwbarton, thomie, mpickering, carter
      
      GHC Trac Issues: #15178
      
      Differential Revision: https://phabricator.haskell.org/D4684
      8ed8b037
  25. Jun 03, 2018
  26. May 18, 2018
    • Simon Peyton Jones's avatar
      Orient TyVar/TyVar equalities with deepest on the left · 2bbdd00c
      Simon Peyton Jones authored
      Trac #15009 showed that, for Given TyVar/TyVar equalities, we really
      want to orient them with the deepest-bound skolem on the left. As it
      happens, we also want to do the same for Wanteds, but for a different
      reason (more likely to be touchable).  Either way, deepest wins:
      see TcUnify Note [Deeper level on the left].
      
      This observation led me to some significant changes:
      
      * A SkolemTv already had a TcLevel, but the level wasn't really being
        used.   Now it is!
      
      * I updated added invariant (SkolInf) to TcType
        Note [TcLevel and untouchable type variables], documenting that
        the level number of all the ic_skols should be the same as the
        ic_tclvl of the implication
      
      * FlatSkolTvs and FlatMetaTvs previously had a dummy level-number of
        zero, which messed the scheme up.   Now they get a level number the
        same way as all other TcTyVars, instead of being a special case.
      
      * To make sure that FlatSkolTvs and FlatMetaTvs are untouchable (which
        was previously done via their magic zero level) isTouchableMetaTyVar
        just tests for those two cases.
      
      * TcUnify.swapOverTyVars is the crucial orientation function; see the
        new Note [TyVar/TyVar orientation].  I completely rewrote this function,
        and it's now much much easier to understand.
      
      I ended up doing some related refactoring, of course
      
      * I noticed that tcImplicitTKBndrsX and tcExplicitTKBndrsX were doing
        a lot of useless work in the case where there are no skolems; I
        added a fast-patch
      
      * Elminate the un-used tcExplicitTKBndrsSig; and thereby get rid of
        the higher-order parameter to tcExpliciTKBndrsX.
      
      * Replace TcHsType.emitTvImplication with TcUnify.checkTvConstraints,
        by analogy with TcUnify.checkConstraints.
      
      * Inline TcUnify.buildImplication into its only call-site in
        TcUnify.checkConstraints
      
      * TcS.buildImplication becomes TcS.CheckConstraintsTcS, with a
        simpler API
      
      * Now that we have NoEvBindsVar we have no need of termEvidenceAllowed;
        nuke the latter, adding Note [No evidence bindings] to TcEvidence.
      2bbdd00c
  27. May 16, 2018
    • Ryan Scott's avatar
      Fix #15073 by suggesting UnboxedTuples in an error message · 0c7db226
      Ryan Scott authored
      Under certain circumstances, `GeneralizedNewtypeDeriving`
      can emit code which uses unboxed tuple types, but if `UnboxedTuples`
      wasn't enabled, the error message that GHC gave didn't make it very
      clear that it could be worked around by explicitly enabling the
      extension. Easily fixed.
      
      Test Plan: make test TEST=T15073
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: simonpj, thomie, carter
      
      GHC Trac Issues: #15073
      
      Differential Revision: https://phabricator.haskell.org/D4620
      0c7db226
  28. May 14, 2018
    • Ryan Scott's avatar
      Fix #14875 by introducing PprPrec, and using it · 21e1a00c
      Ryan Scott authored
      Trying to determine when to insert parentheses during TH
      conversion is a bit of a mess. There is an assortment of functions
      that try to detect this, such as:
      
      * `hsExprNeedsParens`
      * `isCompoundHsType`
      * `hsPatNeedsParens`
      * `isCompoundPat`
      * etc.
      
      To make things worse, each of them have slightly different semantics.
      Plus, they don't work well in the presence of explicit type
      signatures, as #14875 demonstrates.
      
      All of these problems can be alleviated with the use of an explicit
      precedence argument (much like what `showsPrec` currently does). To
      accomplish this, I introduce a new `PprPrec` data type, and define
      standard predences for things like function application, infix
      operators, function arrows, and explicit type signatures (that last
      one is new). I then added `PprPrec` arguments to the various
      `-NeedsParens` functions, and use them to make smarter decisions
      about when things need to be parenthesized.
      
      A nice side effect is that functions like `isCompoundHsType` are
      now completely unneeded, since they're simply aliases for
      `hsTypeNeedsParens appPrec`. As a result, I did a bit of refactoring
      to remove these sorts of functions. I also did a pass over various
      utility functions in GHC for constructing AST forms and used more
      appropriate precedences where convenient.
      
      Along the way, I also ripped out the existing `TyPrec`
      data type (which was tailor-made for pretty-printing `Type`s) and
      replaced it with `PprPrec` for consistency.
      
      Test Plan: make test TEST=T14875
      
      Reviewers: alanz, goldfire, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14875
      
      Differential Revision: https://phabricator.haskell.org/D4688
      21e1a00c
  29. Apr 01, 2018
    • 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
  30. Mar 25, 2018
    • Ryan Scott's avatar
      Fix #14916 with an additional validity check in deriveTyData · 20f14b4f
      Ryan Scott authored
      Manually-written instances and standalone-derived instances
      have the benefit of having the `checkValidInstHead` function run over
      them, which catches manual instances of built-in types like `(~)` and
      `Coercible`. However, instances generated from `deriving` clauses
      weren't being passed through `checkValidInstHead`, leading to
      confusing results as in #14916.
      
      `checkValidInstHead` also has additional validity checks for language
      extensions like `FlexibleInstances` and `MultiParamTypeClasses`. Up
      until now, GHC has never required these language extensions for
      `deriving` clause, so to avoid unnecessary breakage, I opted to
      suppress these language extension checks for `deriving` clauses, just
      like we currently suppress them for `SPECIALIZE instance` pragmas.
      
      Test Plan: make test TEST=T14916
      
      Reviewers: goldfire, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14916
      
      Differential Revision: https://phabricator.haskell.org/D4501
      20f14b4f
    • Ryan Scott's avatar
      Fix two pernicious bugs in DeriveAnyClass · 98930426
      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
      98930426
  31. Mar 23, 2018
    • Ryan Scott's avatar
      Special-case record fields ending with hash when deriving Read · d5577f44
      Ryan Scott authored
      Summary:
      In commit dbd81f7e, a
      regression was inadvertently introduced which caused derived `Read`
      instances for record data types with fields ending in a `#` symbol
      (using `MagicHash`) would no longer parse on valid output. This
      is ultimately due to the same reasons as #5041, as we cannot parse
      a field name like `foo#` as a single identifier. We fix this issue
      by employing the same workaround as in #5041: first parse the
      identifier name `foo`, then then symbol `#`.
      
      This is accomplished by the new `readFieldHash` function in
      `GHC.Read`. This will likely warrant a `base-4.11.1.0` release.
      
      Test Plan: make test TEST=T14918
      
      Reviewers: tdammers, hvr, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14918
      
      Differential Revision: https://phabricator.haskell.org/D4502
      d5577f44
  32. Feb 18, 2018
    • Ryan Scott's avatar
      Implement stopgap solution for #14728 · 1ede46d4
      Ryan Scott authored
      It turns out that one can produce ill-formed Core by
      combining `GeneralizedNewtypeDeriving`, `TypeInType`, and
      `TypeFamilies`, as demonstrated in #14728. The root of the problem
      is allowing the last parameter of a class to appear in a //kind// of
      an associated type family, as our current approach to deriving
      associated type family instances simply doesn't work well for that
      situation.
      
      Although it might be possible to properly implement this feature
      today (see https://ghc.haskell.org/trac/ghc/ticket/14728#comment:3
      for a sketch of how this might work), there does not currently exist
      a performant implementation of the algorithm needed to accomplish
      this. Until such an implementation surfaces, we will make this corner
      case of `GeneralizedNewtypeDeriving` an error.
      
      Test Plan: make test TEST="T14728a T14728b"
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14728
      
      Differential Revision: https://phabricator.haskell.org/D4402
      1ede46d4
  33. Feb 01, 2018
    • Ryan Scott's avatar
      Sequester deriving-related validity check into cond_stdOK · 1a911f21
      Ryan Scott authored
      Currently, any standalone-derived instance must satisfy the
      property that the tycon of the data type having an instance being
      derived for it must be either a normal ADT tycon or a data family
      tycon. But there are several other primitive tycons—such as `(->)`,
      `Int#`, and others—which cannot have standalone-derived instances
      (via the `anyclass` strategy) as a result of this check! See
      https://ghc.haskell.org/trac/ghc/ticket/13154#comment:8 for an
      example of where this overly conservative restriction bites.
      
      Really, this validity check only makes sense in the context of
      `stock` deriving, where we need the property that the tycon is that
      of a normal ADT or a data family in order to inspect its data
      constructors. Other deriving strategies don't require this validity
      check, so the most sensible way to fix this error is to move the
      logic of this check into `cond_stdOK`, which is specific to
      `stock` deriving.
      
      This makes progress towards fixing (but does not entirely fix)
      
      Test Plan: make test TEST=T13154a
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #13154
      
      Differential Revision: https://phabricator.haskell.org/D4337
      1a911f21
  34. Jan 21, 2018
  35. Jan 18, 2018
    • Ryan Scott's avatar
      Fix #14681 and #14682 with precision-aimed parentheses · 575c009d
      Ryan Scott authored
      It turns out that `Convert` was recklessly leaving off
      parentheses in two places:
      
      * Negative numeric literals
      * Patterns in lambda position
      
      This patch fixes it by adding three new functions, `isCompoundHsLit`,
      `isCompoundHsOverLit`, and `isCompoundPat`, and using them in the
      right places in `Convert`. While I was in town, I also sprinkled
      `isCompoundPat` among some `Pat`-constructing functions in `HsUtils`
      to help avoid the likelihood of this problem happening in other
      places. One of these places is in `TcGenDeriv`, and sprinkling
      `isCompountPat` there fixes #14682
      
      Test Plan: make test TEST="T14681 T14682"
      
      Reviewers: alanz, goldfire, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14681, #14682
      
      Differential Revision: https://phabricator.haskell.org/D4323
      575c009d
  36. Jan 04, 2018
    • Ryan Scott's avatar
      Make typeToLHsType produce kind signatures for tycon applications · 649e7772
      Ryan Scott authored
      Summary:
      `GeneralizedNewtypeDeriving` generates calls to `coerce`
      which take visible type arguments. These types must be produced by
      way of `typeToLHsType`, which converts a `Type` to an `LHsType`.
      However, `typeToLHsType` was leaving off important kind information
      when a `Type` contained a poly-kinded tycon application, leading to
      incorrectly generated code in #14579.
      
      This fixes the issue by tweaking `typeToLHsType` to generate
      explicit kind signatures for tycon applications. This makes the
      generated code noisier, but at least the program from #14579 now
      works correctly.
      
      Test Plan: make test TEST=T14579
      
      Reviewers: simonpj, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14579
      
      Differential Revision: https://phabricator.haskell.org/D4264
      649e7772
Loading