GitLab

Previously, we checked the number of patterns in a data instances
for all data families whose kind did not end in a kind variable.
But, of course, undersaturating instances can happen even without
the kind ending in a kind variable. So I've omitted the arity check.
Data families aren't as particular about their arity as type families
are (because data families can be undersaturated). Still, this change
degrades error messages when instances don't have the right arity;
now, instead of reporting a simple mismatch in the number of patterns,
GHC reports kind errors. The new errors are fully accurate, but perhaps
not as easy to work with. Still, with the new flexibility of allowing
data family instances with varying numbers of patterns, I don't see
a better way.

This commit also improves source fidelity in some error messages,
requiring more changes than really are necessary. But without these
changes, error messages around mismatched associated instance heads
were poor.

test cases: indexed-types/should_compile/T14045,
            indexed-types/should_fail/T14045a

Summary:
Some recent commits happened to fix other issues:

* c2417b87 fixed #13601 and #13780
* 8e15e3d3 fixed the original program in #13877

Let's add regression tests for each of these to ensure they stay fixed.

Test Plan: make test TEST="T13601 T13780a T13780c T13877"

Reviewers: goldfire, bgamari, austin

Reviewed By: bgamari

Subscribers: rwbarton, thomie

GHC Trac Issues: #13601, #13780, #13877

Differential Revision: https://phabricator.haskell.org/D3794

The big change here is to fix an outright bug in flattening of Givens,
albeit one that is very hard to exhibit.  Suppose we have the constraint
    forall a. (a ~ F b) => ..., (forall c. ....(F b)...) ...

Then
 - we'll flatten the (F) b to a fsk, say  (F b ~ fsk1)
 - we'll rewrite the (F b) inside the inner implication to 'fsk1'
 - when we leave the outer constraint we are suppose to unflatten;
   but that fsk1 will still be there
 - if we re-simplify the entire outer implication, we'll re-flatten
   the Given (F b) to, say, (F b ~ fsk2)
Now we have two fsks standing for the same thing, and that is very
wrong.

Solution: make fsks behave more like fmvs:
 - A flatten-skolem is now a MetaTyVar, whose MetaInfo is FlatSkolTv
 - We "fill in" that meta-tyvar when leaving the implication
 - The old FlatSkol form of TcTyVarDetails is gone completely
 - We track the flatten-skolems for the current implication in
   a new field of InertSet, inert_fsks.

See Note [The flattening story] in TcFlatten.

In doing this I found various other things to fix:

* I removed the zonkSimples from TcFlatten.unflattenWanteds; it wasn't
  needed.   But I added one in TcSimplify.floatEqualities, which does
  the zonk precisely when it is needed.

* Trac #13674 showed up a case where we had
     - an insoluble Given,   e.g.  a ~ [a]
     - the same insoluble Wanted   a ~ [a]
  We don't use the Given to rewwrite the Wanted (obviously), but
  we therefore ended up reporting
      Can't deduce (a ~ [a]) from (a ~ [a])
  which is silly.

  Conclusion: when reporting errors, make the occurs check "win"
  See Note [Occurs check wins] in TcErrors

Based on my quick search, we don't have a test
that verifies that we check the type family instances of
currently compiled module against direct or indirect
dependencies.

This adds two tests: for a direct dependency and
for an indirect dependency.

I also added a comment to make it clear what the 'Over'
test tests.

Other than completeness, it makes sense to have these
tests because if you look at
Note [The type family instance consistency story] in FamInsts
these cases are checked through different mechanisms.

Test Plan: new tests

Reviewers: simonmar, rwbarton, simonpj, austin, bgamari

Reviewed By: simonpj, bgamari

Subscribers: thomie

GHC Trac Issues: #13719

Differential Revision: https://phabricator.haskell.org/D3602

Summary:
After 8136a5cb (#11450), if you have
a class with an associated type:

```
class C a where
  type T a b
```

And you try to create an instance of `C` like this:

```
instance C Int where
  type T Int Char = Bool
```

Then it is rejected, since you're trying to instantiate the variable ``b`` with
something other than a type variable. But this restriction proves quite
onerous in practice, as it prevents you from doing things like this:

```
class C a where
  type T a (b :: Identity c) :: c

instance C Int where
  type T Int ('Identity x) = x
```

You have to resort to an auxiliary type family in order to define this now,
which becomes extremely tiring. This lifts this restriction and fixes #13398,
in which it was discovered that adding this restriction broke code in the wild.

Test Plan: ./validate

Reviewers: simonpj, bgamari, austin

Reviewed By: simonpj

Subscribers: rwbarton, thomie

Differential Revision: https://phabricator.haskell.org/D3302

The script I used is included as testsuite/driver/kill_extra_files.py,
though at this point it is for mostly historical interest.

Some of the tests in libraries/hpc relied on extra_files.py, so this
commit includes an update to that submodule.

One test in libraries/process also relies on extra_files.py, but we
cannot update that submodule so easily, so for now we special-case it
in the test driver.

This fixes the easy part of #7102 by removing the logic that lets the
user replace a type family instance with a new one with the same LHS.
As discussed on that ticket, this is unsound in general. Better to have
the user redefine the type family from scratch.

The example from comment:7 involving loading modules into ghci is not
fixed yet; it actually doesn't rely on the instances having the same LHS.
This commit adds an expect_broken test for that example as well.

Test Plan: T7102a for the fix; T7102 is the test not fixed yet

Reviewers: dfeuer, austin, bgamari, goldfire

Reviewed By: dfeuer

Subscribers: dfeuer, thomie

Differential Revision: https://phabricator.haskell.org/D2994

The `clean_cmd` and `extra_clean` setup functions don't do anything.
Remove them from .T files.

Created using https://github.com/thomie/refactor-ghc-testsuite. This
diff is a test for the .T-file parser/processor/pretty-printer in that
repository.

    find . -name '*.T' -exec ~/refactor-ghc-testsuite/Main "{}" \;

Tests containing inline comments or multiline strings are not modified.

Preparation for #12223.

Test Plan: Harbormaster

Reviewers: austin, hvr, simonmar, mpickering, bgamari

Reviewed By: mpickering

Subscribers: mpickering

Differential Revision: https://phabricator.haskell.org/D3000

GHC Trac Issues: #12223

Up to now we've had a rule that a TyVar can't apppear in a type
seen by the type checker; they should all be TcTyVars.  But:

a) With -XTypeInType it becomes much harder to exclude them;
   see Note [TcTyVars in the typechecker] in TcType.

b) It's unnecessary to exculde them; instead we can just treat
   a TyVar just like vanillaSkolemTv.

This is what was causing an ASSERT error in
indexed-types/should_fail/T12041, reported in Trac #12826.

That patch allows a TyVar in a TcType.  The most significant
change is to make Var.tcTyVarDetails return vanillaSkolemTv.
In fact it already did, but (a) it was not documented, and
(b) we never exploited it.  Now we rely on it.

When `GeneralizedNewtypeDeriving` is used with a type class that has no
methods, it will generate a redundant context, and as a result, it can
trigger warnings when compiled with `-Wredundant-constraints`. This is a
simple change in behavior to check beforehand if a class has methods
when deriving it with GND, and if it has no methods, avoid inferring the
redundant context.

Beware that the test for #6088, which used to be expected to fail, now
compiles without issue since it doesn't infer a problematic instance
context.

Thanks to Simon Peyton Jones for doing the necessary refactoring in
f05d685a.

Fixes #12814.

Test Plan: ./validate

Reviewers: goldfire, rwbarton, simonpj, austin, bgamari

Reviewed By: simonpj

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D2692

GHC Trac Issues: #12814

These ones test the variations in coment:15 of the ticket

I don't know why some ways were omitted before.

The previous pretty-printer didn't account for partially
applied equalities, causing Trac #12041

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.

See Note [Prevent unification with type families]
in TcUnify for the details.

Previously, checkTauTvUpdate, used in the eager unifier (TcUnify)
right before writing to a metavar, refused to write a metavar to
a type involving type functions. The reason for this was given
in a Note, but the Note didn't make all that much sense and even
admitted that it was a bit confused. The Note, in turn, referred
to another Note, which it was quite sceptical of, as well.

The type-family check was slowing down performance, so I tried
removing it, running the tests referred to in the Note. The tests
all passed without the check. Looking at more test results, I
saw several error messages improve without the check, and some cases
where GHC looped (T7788, in particular) it now doesn't.

So, all in all, quite a win: Two hairy Notes removed, several lines
of code removed, better performance, and improved output.

[skip ci]

Fixes Trac #11136.  We should check arity before
doing tcTyClTyVars, because the latter crahes if
the arity isn't right.

This fixes #10817 and #10899. A knock-on effect is that we must
now remember locations of associated type defaults for error
messages during validity checking. This isn't too bad, but it
increases the size of the diff somewhat.

Test cases: indexed-types/should_fail/T108{17,99}

No point in pretending other compilers can use the GHC testsuite. This
makes the *.T files a bit shorter.

This makes it easier to grep for expect_fail in the tests directory.

Differential Revision: https://phabricator.haskell.org/D965

Summary:
It's kind of buggy, c.f. #10182, and isn't motivated by any
real world programs, so we're going to get rid of it (despite
it being handy for GHC test cases.)
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: simonpj, austin

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D862

This implements (roughly) the plan put forward in comment:14:ticket:7788,
fixing #7788, #8550, #9554, #10139, and addressing concerns raised in #10079.
There are some regressions w.r.t. GHC 7.8, but only with pathological type
families (like F a = F a). This also (hopefully -- don't have a test case)
fixes #10158. Unsolved problems include #10184 and #10185, which are both
known deficiencies of the approach used here.

As part of this change, the plumbing around detecting infinite loops has
changed. Instead of -fcontext-stack and -ftype-function-depth, we now have
one combined -freduction-depth parameter. Setting it to 0 disbales the
check, which is now the recommended way to get (terminating) code to
typecheck in releases. (The number of reduction steps may well change between
minor GHC releases!)

This commit also introduces a new IntWithInf type in BasicTypes
that represents an integer+infinity. This type is used in a few
places throughout the code.

Tests in
  indexed-types/should_fail/T7788
  indexed-types/should_fail/T8550
  indexed-types/should_fail/T9554
  indexed-types/should_compile/T10079
  indexed-types/should_compile/T10139
  typecheck/should_compile/T10184  (expected broken)
  typecheck/should_compile/T10185  (expected broken)

This commit also changes performance testsuite numbers, for the better.

The purpose of silent superclass parameters was to solve the
awkward problem of superclass dictinaries being bound to bottom.
See THE PROBLEM in Note [Recursive superclasses] in TcInstDcls

Although the silent-superclass idea worked,

  * It had non-local consequences, and had effects even in Haddock,
    where we had to discard silent parameters before displaying
    instance declarations

  * It had unexpected peformance costs, shown up by Trac #3064 and its
    test case.  In monad-transformer code, when constructing a Monad
    dictionary you had to pass an Applicative dictionary; and to
    construct that you neede a Functor dictionary. Yet these extra
    dictionaries were often never used.  (All this got much worse when
    we added Applicative as a superclass of Monad.) Test T3064
    compiled *far* faster after silent superclasses were eliminated.

  * It introduced new bugs.  For example SilentParametersOverlapping,
    T5051, and T7862, all failed to compile because of instance overlap
    directly because of the silent-superclass trick.

So this patch takes a new approach, which I worked out with Dimitrios
in the closing hours before Christmas.  It is described in detail
in THE PROBLEM in Note [Recursive superclasses] in TcInstDcls.

Seems to work great!

Quite a bit of knock-on effect

 * The main implementation work is in tcSuperClasses in TcInstDcls
   Everything else is fall-out

 * IdInfo.DFunId no longer needs its n-silent argument
   * Ditto IDFunId in IfaceSyn
   * Hence interface file format changes

 * Now that DFunIds do not have silent superclass parameters, printing
   out instance declarations is simpler. There is tiny knock-on effect
   in Haddock, so that submodule is updated

 * I realised that when computing the "size of a dictionary type"
   in TcValidity.sizePred, we should be rather conservative about
   type functions, which can arbitrarily increase the size of a type.
   Hence the new datatype TypeSize, which has a TSBig constructor for
   "arbitrarily big".

 * instDFunType moves from TcSMonad to Inst

 * Interestingly, CmmNode and CmmExpr both now need a non-silent
   (Ord r) in a couple of instance declarations. These were previously
   silent but must now be explicit.

 * Quite a bit of wibbling in error messages

See comments in #8984. This takes back the fix for #6088.

This reverts commit 5760eb59
because the very same test was already added via
5eebd990 and is causing
`./validate` to fail due to "framework failure".

Test T2239 actually succeeds without impredicativity, because of the new co/contra subsumption check

This patch fixes Trac #9580, in which the Coercible machinery succeeded
even though the relevant data constructor was not in scope.

As usual I got dragged into a raft of refactoring changes,
all for the better.

* Delete TcEvidence.coercionToTcCoercion (now unused)

* Move instNewTyConTF_maybe, instNewTyCon_maybe to FamInst,
  and rename them to tcInstNewTyConTF_maybe, tcInstNewTyCon
  (They both return TcCoercions.)

* tcInstNewTyConTF_maybe also gets more convenient type,
  which improves TcInteract.getCoercibleInst

* Define FamInst.tcLookupDataFamInst, and use it in TcDeriv,
  (as well as in tcInstNewTyConTF_maybe)

* Improve error report for Coercible errors, when data familes
  are involved  Another use of tcLookupDataFamInst

* In TcExpr.tcTagToEnum, use tcLookupDataFamInst to replace
  local hacky code

* Fix Coercion.instNewTyCon_maybe and Type.newTyConInstRhs to deal
  with eta-reduced newtypes, using
  (new) Type.unwrapNewTyConEtad_maybe and (new) Type.applyTysX

Some small refactoring of TcSMonad.matchFam.

Came up on GHC users list

This patch corrects an egregious error introduced by:

  commit 022f8750
  Author: Simon Peyton Jones <simonpj@microsoft.com>
  Date:   Thu May 15 16:07:04 2014 +0100

    Refactoring around TyCon.isSynTyCon

    * Document isSynTyCon better
    * Add isTypeSyonymTyCon for regular H98 type synonyms
    * Use isTypeSynonymTyCon rather than isSynTyCon where
      the former is really intended

At this particular spot in TcValidity we really do mean
isSynTyCon and not isTypeSynonymTyCon.

Fixes Trac #9433