1. 18 May, 2018 3 commits
    • 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
    • Simon Peyton Jones's avatar
      Tiny refactor · efe40544
      Simon Peyton Jones authored
      efe40544
    • Simon Peyton Jones's avatar
      Comments only · 797a4623
      Simon Peyton Jones authored
      797a4623
  2. 17 May, 2018 1 commit
    • Simon Marlow's avatar
      Fix GHCi space leaks (#15111) · f27e4f62
      Simon Marlow authored
      Summary:
      There were a number of leaks causing previously loaded modules to be
      retained after a new `:load`.  This fixes enough leaks to get the
      tests to pass from D4658.
      
      Test Plan: See new tests in D4658
      
      Reviewers: niteria, bgamari, simonpj, erikd
      
      Subscribers: thomie, carter
      
      GHC Trac Issues: #15111
      
      Differential Revision: https://phabricator.haskell.org/D4659
      f27e4f62
  3. 16 May, 2018 2 commits
    • 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
    • Ryan Scott's avatar
      Fix #15039 by pretty-printing equalities more systematically · 99f8cc84
      Ryan Scott authored
      GHC previously had a handful of special cases for
      pretty-printing equalities in a more user-friendly manner, but they
      were far from comprehensive (see #15039 for an example of where this
      fell apart).
      
      This patch makes the pretty-printing of equalities much more
      systematic. I've adopted the approach laid out in
      https://ghc.haskell.org/trac/ghc/ticket/15039#comment:4, and updated
      `Note [Equality predicates in IfaceType]` accordingly. We are now
      more careful to respect the properties of the
      `-fprint-explicit-kinds` and `-fprint-equality-relations` flags,
      which led to some improvements in error message outputs.
      
      Along the way, I also tweaked the error-reporting machinery not to
      print out the type of a typed hole when the type is an unlifted
      equality, since it's kind (`TYPE ('TupleRep '[])`) was more
      confusing than anything.
      
      Test Plan: make test TEST="T15039a T15039b T15039c T15039d"
      
      Reviewers: simonpj, goldfire, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #15039
      
      Differential Revision: https://phabricator.haskell.org/D4696
      99f8cc84
  4. 15 May, 2018 2 commits
    • Artem Pelenitsyn's avatar
      Less Tc inside simplCore (Phase 1 for #14391) · bb3fa2d1
      Artem Pelenitsyn authored
      Simplifier depends on typechecker in two points: `thNameToGhcName`
      (`lookupThName_maybe`, in particular)  and `lookupGlobal`. We want to
      cut the ties in two steps.
      
      1. (Presented in this commit), reimplement both functions in a way that
      doesn't use typechecker.
      
      2. (Should follow), do code moving: a) `lookupGlobal` should go in some
      typechecker-free place; b) `thNameToGhcName` should leave simplifier,
      because it is not used there at all (probably, it should be placed
      somewhere where `GhcPlugins` can see it -- this is suggested by Joachim
      on Trac).
      
      Details
      =======
      
      We redesigned lookup interface a bit so that it exposes some
      `IO`-equivalents of `Tc`-features in use.
      
      First, `CoreMonad.hs` still calls `lookupGlobal` which is no longer
      bound to the typechecker monad, but still resides in `TcEnv.hs` — it
      should be moved out of Tc-land at some point (“Phase 2”) in the
      future in order to achieve its part of the #14391's goal.
      
      Second, `lookupThName_maybe` is eliminated from `CoreMonad.hs`
      completely; this already achieves its part of the goal of #14391. Its
      client, though, `thNameToGhcName`, is better to be moved in the future
      also, for it is not used in the `CoreMonad.hs` (or anywhere else)
      anyway. Joachim suggested “any module reexported by GhcPlugins (or
      maybe even that module itself)”.
      
      As a side goal, we removed `initTcForLookup` which was instrumental for
      the past version of `lookupGlobal`. This, in turn, called for pushing
      some more parts of the lookup interface from the `Tc`-monad to `IO`,
      most notably, adding `IO`-version of `lookupOrig` and pushing
      `dataConInfoPtrToName` to `IO`. The `lookupOrig` part, in turn,
      triggered a slight redesign of name cache updating interface: we now
      have both, `updNameCacheIO` and `updNameCacheTc`, both accepting `mod`
      and `occ` to force them inside, instead of more error-prone outside
      before. But all these hardly have to do anything with #14391, mere
      refactoring.
      
      Reviewers: simonpj, nomeata, bgamari, hvr
      
      Reviewed By: simonpj, bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14391
      
      Differential Revision: https://phabricator.haskell.org/D4503
      bb3fa2d1
    • Simon Peyton Jones's avatar
      Tidy up error suppression · f49f90bb
      Simon Peyton Jones authored
      Trac #15152 showed that when a flag turned an error into a warning, we
      were still (alas) suppressing subequent errors; includign their
      essential addTcEvBind.  That led (rightly) to a Lint error.
      
      This patch fixes it, and incidentally tidies up an ad-hoc special
      case of out-of-scope variables (see the old binding for
      'out_of_scope_killer' in 'tryReporters').
      
      No test, because the problem was only shown up when turning
      inaccessible code into a warning.
      f49f90bb
  5. 14 May, 2018 1 commit
    • 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
  6. 13 May, 2018 1 commit
  7. 08 May, 2018 1 commit
  8. 05 May, 2018 1 commit
  9. 27 Apr, 2018 4 commits
    • Simon Peyton Jones's avatar
      Better linting for types · 6da5b877
      Simon Peyton Jones authored
      Trac #15057 described deficiencies in the linting for types
      involving type synonyms.  This patch fixes an earlier attempt.
      
      The moving parts are desrcribed in
        Note [Linting type synonym applications]
      
      Not a big deal.
      6da5b877
    • Simon Peyton Jones's avatar
      Make out-of-scope errors more prominent · 08003e7f
      Simon Peyton Jones authored
      Generally, when the type checker reports an error, more serious
      ones suppress less serious ones.
      
      A "variable out of scope" error is arguably the most serious of all,
      so this patch moves it to the front of the list instead of the end.
      
      This patch also fixes Trac #14149, which had
      -fdefer-out-of-scope-variables, but also had a solid type error.
      As things stood, the type error was not reported at all, and
      compilation "succeeded" with error code 0.  Yikes.
      
      Note that
      
      - "Hole errors" (including out of scope) are never suppressed.
        (maybeReportHoleError vs maybeReportError in TcErorrs)
        They can just get drowned by the noise.
      
      - But with the new orientation, out of scope errors will suppress
        type errors.  That would be easy to change.
      08003e7f
    • Simon Peyton Jones's avatar
      Refactor tcExtendLocalFamInst a bit · 0c01224b
      Simon Peyton Jones authored
      This patch just pulls out FamInst.loadDependentFamInstModules
      as a separate function, and adds better comments.
      
      Provoked by Trac #14759, comment:10.
      0c01224b
    • Alan Zimmerman's avatar
      TTG : complete for balance of hsSyn AST · c3823cba
      Alan Zimmerman authored
      Summary:
      - remove PostRn/PostTc fields
      - remove the HsVect In/Out distinction for Type, Class and Instance
      - remove PlaceHolder in favour of NoExt
      - Simplify OutputableX constraint
      
      Updates haddock submodule
      
      Test Plan: ./validate
      
      Reviewers: goldfire, bgamari
      
      Subscribers: goldfire, thomie, mpickering, carter
      
      Differential Revision: https://phabricator.haskell.org/D4625
      c3823cba
  10. 20 Apr, 2018 2 commits
    • 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
    • Ryan Scott's avatar
      Lint types in newFamInst · 257c13d8
      Ryan Scott authored
      We weren't linting the types used in `newFamInst`, which
      might have been why #15012 went undiscovered for so long. Let's fix
      that.
      
      One has to be surprisingly careful with expanding type synonyms in
      `lintType`, since in the offending program (simplified):
      
      ```lang=haskell
      type FakeOut a = Int
      
      type family TF a
      type instance TF Int = FakeOut a
      ```
      
      If one expands type synonyms, then `FakeOut a` will expand to
      `Int`, which masks the issue (that `a` is unbound). I added an
      extra Lint flag to configure whether type synonyms should be
      expanded or not in Lint, and disabled this when calling `lintTypes`
      from `newFamInst`.
      
      As evidence that this works, I ran it on the offending program
      from #15012, and voilà:
      
      ```
      $ ghc3/inplace/bin/ghc-stage2 Bug.hs -dcore-lint
      [1 of 1] Compiling Foo              ( Bug.hs, Bug.o )
      ghc-stage2: panic! (the 'impossible' happened)
        (GHC version 8.5.20180417 for x86_64-unknown-linux):
              Core Lint error
        <no location info>: warning:
            In the type ‘... (Rec0 (FakeOut b_a1Qt))))’
            @ b_a1Qt is out of scope
      ```
      
      Test Plan: make test TEST=T15057
      
      Reviewers: simonpj, goldfire, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: thomie, carter
      
      GHC Trac Issues: #15057
      
      Differential Revision: https://phabricator.haskell.org/D4611
      257c13d8
  11. 19 Apr, 2018 1 commit
    • Ryan Scott's avatar
      Fix #15012 with a well-placed use of Any · b08a6d75
      Ryan Scott authored
      Previously, derived `Generic1` instances could have associated `Rep1`
      type family instances with unbound variables, such as in the following
      example:
      
      ```lang=haskell
      data T a = MkT (FakeOut a) deriving Generic1
      type FakeOut a = Int
      
      ==>
      
      instance Generic1 T where
        type Rep1 T = ... (Rec0 (FakeOut a))
      ```
      
      Yikes! To avoid this, we simply map the last type variable in a
      derived `Generic1` instance to `Any`.
      
      Test Plan: make test TEST=T15012
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: simonpj, thomie, carter
      
      GHC Trac Issues: #15012
      
      Differential Revision: https://phabricator.haskell.org/D4602
      b08a6d75
  12. 13 Apr, 2018 1 commit
    • Alan Zimmerman's avatar
      TTG for HsBinds and Data instances Plan B · b1386942
      Alan Zimmerman authored
      Summary:
      - Add the balance of the TTG extensions for hsSyn/HsBinds
      
      - Move all the (now orphan) data instances into hsSyn/HsInstances and
      use TTG Data instances Plan B
      https://ghc.haskell.org/trac/ghc/wiki/ImplementingTreesThatGrow/Instances#PLANB
      
      Updates haddock submodule.
      
      Illustrative numbers
      
      Compiling HsInstances before using Plan B.
      
      Max residency ~ 5G
      <<ghc: 629,864,691,176 bytes, 5300 GCs,
             321075437/1087762592 avg/max bytes residency (23 samples),
             2953M in use, 0.000 INIT (0.000 elapsed),
             383.511 MUT (384.986 elapsed), 37.426 GC (37.444 elapsed) :ghc>>
      
      Using Plan B
      
      Max residency 1.1G
      
      <<ghc: 78,832,782,968 bytes, 2884 GCs,
             222140352/386470152 avg/max bytes residency (34 samples),
             1062M in use, 0.001 INIT (0.001 elapsed),
             56.612 MUT (62.917 elapsed), 32.974 GC (32.923 elapsed) :ghc>>
      
      Test Plan: ./validate
      
      Reviewers: shayan-najd, goldfire, bgamari
      
      Subscribers: goldfire, thomie, mpickering, carter
      
      Differential Revision: https://phabricator.haskell.org/D4581
      b1386942
  13. 10 Apr, 2018 1 commit
  14. 09 Apr, 2018 1 commit
  15. 07 Apr, 2018 1 commit
  16. 02 Apr, 2018 1 commit
    • Richard Eisenberg's avatar
      Fix #14991. · d8d4266b
      Richard Eisenberg authored
      It turns out that solveEqualities really does need to use simpl_top.
      I thought that solveWanteds would be enough, and no existing test
      case showed up the different. #14991 shows that we need simpl_top.
      Easy enough to fix.
      
      test case: dependent/should_compile/T14991
      d8d4266b
  17. 01 Apr, 2018 2 commits
    • Richard Eisenberg's avatar
      Clarify comments around dropping Derived constraints · 1845d1bc
      Richard Eisenberg authored
      [skip ci]
      1845d1bc
    • 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
  18. 27 Mar, 2018 1 commit
  19. 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
  20. 25 Mar, 2018 4 commits
    • 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
    • Adam Gundry's avatar
      Fix panic on module re-exports of DuplicateRcordFields · fb462f94
      Adam Gundry authored
      Test Plan: new test overloadedrecflds/should_fail/T14953
      
      Reviewers: mpickering, simonpj, bgamari
      
      Reviewed By: bgamari
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14953
      
      Differential Revision: https://phabricator.haskell.org/D4527
      fb462f94
    • 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
    • Alec Theriault's avatar
      Support adding objects from TH · ceb91477
      Alec Theriault authored
      The user facing TH interface changes are:
      
        * 'addForeignFile' is renamed to 'addForeignSource'
        * 'qAddForeignFile'/'addForeignFile' now expect 'FilePath's
        * 'RawObject' is now a constructor for 'ForeignSrcLang'
        * 'qAddTempFile'/'addTempFile' let you request a temporary file
          from the compiler.
      
      Test Plan: unsure about this, added a TH test
      
      Reviewers: goldfire, bgamari, angerman
      
      Reviewed By: bgamari, angerman
      
      Subscribers: hsyl20, mboes, carter, simonmar, bitonic, ljli, rwbarton, thomie
      
      GHC Trac Issues: #14298
      
      Differential Revision: https://phabricator.haskell.org/D4217
      ceb91477
  21. 23 Mar, 2018 2 commits
    • Ryan Scott's avatar
      Allow PartialTypeSignatures in standalone deriving contexts · affdea82
      Ryan Scott authored
      Summary:
      At its core, this patch is a simple tweak that allows a user
      to write:
      
      ```lang=haskell
      deriving instance _ => Eq (Foo a)
      ```
      
      Which is functionally equivalent to:
      
      ```lang=haskell
      data Foo a = ...
        deriving Eq
      ```
      
      But with the added flexibility that `StandaloneDeriving` gives you
      (namely, the ability to use it anywhere, not just in the same module
      that `Foo` was declared in). This fixes #13324, and should hopefully
      address a use case brought up in #10607.
      
      Currently, only the use of a single, extra-constraints wildcard is
      permitted in a standalone deriving declaration. Any other wildcard
      is rejected, so things like
      `deriving instance (Eq a, _) => Eq (Foo a)` are currently forbidden.
      
      There are quite a few knock-on changes brought on by this change:
      
      * The `HsSyn` type used to represent standalone-derived instances
        was previously `LHsSigType`, which isn't sufficient to hold
        wildcard types. This needed to be changed to `LHsSigWcType` as a
        result.
      
      * Previously, `DerivContext` was a simple type synonym for
        `Maybe ThetaType`, under the assumption that you'd only ever be in
        the `Nothing` case if you were in a `deriving` clause. After this
        patch, that assumption no longer holds true, as you can also be
        in this situation with standalone deriving when an
        extra-constraints wildcard is used.
      
        As a result, I changed `DerivContext` to be a proper datatype that
        reflects the new wrinkle that this patch adds, and plumbed this
        through the relevant parts of `TcDeriv` and friends.
      
      * Relatedly, the error-reporting machinery in `TcErrors` also assumed
        that if you have any unsolved constraints in a derived instance,
        then you should be able to fix it by switching over to standalone
        deriving. This was always sound advice before, but with this new
        feature, it's possible to have unsolved constraints even when
        you're standalone-deriving something!
      
        To rectify this, I tweaked some constructors of `CtOrigin` a bit
        to reflect this new subtlety.
      
      This requires updating the Haddock submodule. See my fork at
      https://github.com/RyanGlScott/haddock/commit/067d52fd4be15a1842cbb05f42d9d482de0ad3a7
      
      Test Plan: ./validate
      
      Reviewers: simonpj, goldfire, bgamari
      
      Reviewed By: simonpj
      
      Subscribers: goldfire, rwbarton, thomie, mpickering, carter
      
      GHC Trac Issues: #13324
      
      Differential Revision: https://phabricator.haskell.org/D4383
      affdea82
    • 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
  22. 21 Mar, 2018 2 commits
    • Simon Peyton Jones's avatar
      Allow as-patterns in unidirectional patttern synonyms · 411a97e2
      Simon Peyton Jones authored
      This patch implements GHC Proposal #94, described here
         https://github.com/ghc-proposals/ghc-proposals/pull/94
      
      The effect is simply to lift a totally-undocumented restriction to
      unidirecional pattern synonyms, namely that they can't have as-patterns
      or n+k patterns.
      
      The fix is easy: just remove the checks.
      
      I also took the opportunity to improve the manual entry for
      the semantics of pattern matching for pattern synonyms.
      411a97e2
    • Ryan Scott's avatar
      Fix #14869 by being more mindful of Type vs. Constraint · 49ac3f0f
      Ryan Scott authored
      Summary:
      Before, we were using `isLiftedTypeKind` in `reifyType`
      before checking if a type was `Constraint`. But as it turns out,
      `isLiftedTypeKind` treats `Constraint` the same as `Type`, so every
      occurrence of `Constraint` would be reified as `Type`! To make things
      worse, the documentation for `isLiftedTypeKind` stated that it
      treats `Constraint` //differently// from `Type`, which simply isn't
      true.
      
      This revises the documentation for `isLiftedTypeKind` to reflect
      reality, and defers the `isLiftedTypeKind` check in `reifyType` so
      that it does not accidentally swallow `Constraint`.
      
      Test Plan: make test TEST=T14869
      
      Reviewers: goldfire, bgamari
      
      Reviewed By: goldfire
      
      Subscribers: rwbarton, thomie, carter
      
      GHC Trac Issues: #14869
      
      Differential Revision: https://phabricator.haskell.org/D4474
      49ac3f0f
  23. 19 Mar, 2018 3 commits
    • Ryan Scott's avatar
      Don't permit data types with return kind Constraint · f748c529
      Ryan Scott authored
      Previously, GHC allowed all of the following:
      
      ```lang=haskell
      data Foo1 :: Constraint
      data family Foo2 :: Constraint
      data family Foo3 :: k
      data instance Foo3 :: Constraint
      ```
      
      Yikes! This is because GHC was confusing `Type` with `Constraint`
      due to careless use of the `isLiftedTypeKind` function. To respect
      this distinction, I swapped `isLiftedTypeKind` out for
      `tcIsStarKind`—which does respect this distinction—in the right
      places.
      
      Test Plan: make test TEST="T14048a T14048b T14048c"
      
      Reviewers: bgamari
      
      Reviewed By: bgamari
      
      Subscribers: goldfire, rwbarton, thomie, carter
      
      GHC Trac Issues: #14048
      
      Differential Revision: https://phabricator.haskell.org/D4479
      f748c529
    • Ryan Scott's avatar
      Fix #14934 by including axSub0R in typeNatCoAxiomRules · c3aea396
      Ryan Scott authored
      For some reason, `axSub0R` was left out of `typeNatCoAxiomRules` in
      `TcTypeNats`, which led to disaster when trying to look up `Sub0R` from
      an interface file, as demonstrated in #14934.
      
      The fix is simple—just add `axSub0R` to that list. To help prevent
      an issue like this happening in the future, I added a
      `Note [Adding built-in type families]` to `TcTypeNats`, which
      contains a walkthrough of all the definitions in `TcTypeNats` you
      need to update when adding a new built-in type family.
      
      Test Plan: make test TEST=T14934
      
      Reviewers: bgamari, simonpj
      
      Reviewed By: simonpj
      
      Subscribers: simonpj, rwbarton, thomie, carter
      
      GHC Trac Issues: #14934
      
      Differential Revision: https://phabricator.haskell.org/D4508
      c3aea396
    • Simon Peyton Jones's avatar
      Comments and tiny refactor · 2a3702d8
      Simon Peyton Jones authored
      Related to Ryan's upcoming patch for Trac #14933
      2a3702d8