GitLab

I introduced a silly bug in

  commit 9187d5fb
  Date:   Mon Apr 2 14:55:43 2018 +0100

  Allow unpacking of single-data-con GADTs

that made test T7050 diverge.  This patch fixes it.

Trac #14978 pointed out that single-constructor GADTs should be
unpackable without trouble.

Acutally I realise that even existentials should be unpackable
too, but that's a bit more work, so it's not part of this patch.

See Note [Unpacking GADTs and existentials] in MkId.

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}

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

I found (when investigating Trac #14955) a binding looking like

   Rec { exported_id = ....big...lcl_id...
       ; lcl_id = exported_id }

but bizarrely 'lcl_id' was chosen as the loop breaker, and never
inlined.  It turned out to be an unintended consequence of the
shortOutIndirections code in SimplCore.  Easily fixed.

Pretty-print unused imported names unqualified unconditionally to
make the warning message consistent for ambiguous/unambiguous
identifiers.
Signed-off-by: HE, Tao <sighingnow@gmail.com>

Test Plan: make test TEST="T14881"

Reviewers: bgamari, simonpj

Reviewed By: simonpj

Subscribers: simonpj, rwbarton, thomie, carter

GHC Trac Issues: #14881

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

GHC 8.4.1 is out, so now GHC's support window only extends
back to GHC 8.2. This means we can delete gobs of code that were
only used for GHC 8.0 support. Hooray!

Test Plan: ./validate

Reviewers: bgamari, erikd, dfeuer

Reviewed By: bgamari, dfeuer

Subscribers: alexbiehl, dfeuer, rwbarton, thomie, carter

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

This function is called in lots of places, so if it
every fails it's good to know from where it was called.

This is Simon's patch, part of his `wip/T2893` branch.

Test Plan: Validate

Subscribers: rwbarton, thomie, carter

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

This patch is comments only, plus a minor refactor that
does not change behaviour.

It just records an idea I had for reducing kick-out in the type
constraint-solver.

See Note [Eliminate younger unification variables] in TcUnify.

Sadly, it didn't improve perf, so I've put it aside, leaving
some breadcrumbs for future generations of GHC hackers.

Before this change, for each constructor that we want
to allocate a tag for we would traverse a list of all
the constructors in a datatype to determine which tag
a constructor should get.

This is obviously quadratic and for datatypes with 10k
constructors it actually makes a big difference.

This change implements the plan outlined by @simonpj in
https://mail.haskell.org/pipermail/ghc-devs/2017-October/014974.html
which is basically about using a map and constructing it outside the
loop.

One place where things got a bit awkward was TysWiredIn.hs,
it would have been possible to just assign the tags by hand, but
that seemed error-prone to me, so I decided to go through a map
there as well.

Test Plan:
./validate
On a file with 10k constructors
Before:
   8,130,522,344 bytes allocated in the heap
  Total   time    3.682s  (  3.920s elapsed)
After:
   4,133,478,744 bytes allocated in the heap
  Total   time    2.509s  (  2.750s elapsed)

Reviewers: simonpj, bgamari

Reviewed By: simonpj

Subscribers: goldfire, rwbarton, thomie, simonmar, carter, simonpj

GHC Trac Issues: #14657

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

Trac #13032 pointed out that we sometimes generate unused
bindings for Givens, and (worse still) we can't always discard
them later (we don't drop a case binding unless we can prove
that the scrutinee is non-bottom.

It looks as if this may be a major reason for the performace
problems in #14338 (see comment:29).

This patch fixes the problem at source, by pruning away all the
dead Givens.  See Note [Delete dead Given evidence bindings]

Remarkably, compiler allocation falls by 23% in
perf/compiler/T12227!

I have not confirmed whether this change actualy helps with

See Trac #14626, comment:4.  We want to maintain evaluted-ness
info on Ids into the code generateor for two reasons
(see Note [Preserve evaluated-ness in CorePrep] in CorePrep)

- DataToTag magic
- Potentially using it in the codegen (this is Gabor's
  current work)

But it was all being done very inconsistently, and actually
outright wrong -- the DataToTag magic hasn't been working for
years.

This patch tidies it all up, with Notes to match.

Triggered by thinking about Trac #14596, I found that runRW#
does not need to be a "magic" wired-in Id, now that we have
levity polymorphism.

This patch stops it being wired-in.

This patch fixes Trac #14998, where we eventually decided that
the existential type variables of the signature of a pattern
synonym should not scope over the pattern synonym.

See Note [Pattern synonym existentials do not scope] in TcPatSyn.

Two things here:

* While debugging Trac #14561 I found it hard to understand
  ghcPrimIds and magicIds in MkId.  This patch adds more
  structure and comments.

* I also discovered that ($) no longer needs to be a wiredInId
  because we now have levity polymorphism.  So I took dollarId
  out of MkId; and gave it a levity-polymorphic type in GHC.Base

This bug was shown up by Trac #14561. The deguarer carefully
detects unsaturated and levity-polymorphic uses of primops,
but not of things like unsafeCoerce#.

The fix is simple: see Note [Levity-polymorphic Ids] in Id.

I noticed this while tinkering in haddock. This might be a relict from
ancient times where newtypes wouldn't optimize well.

Reviewers: austin, bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie

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

The idea is described in #14152, and can be summarized: Float the exit
path out of a joinrec, so that the simplifier can do more with it.
See the test case for a nice example.

The floating goes against what the simplifier usually does, hence we
need to be careful not inline them back.

The position of exitification in the pipeline was chosen after a small
amount of experimentation, but may need to be improved. For example,
exitification can allow rewrite rules to fire, but for that it would
have to happen before the `simpl_phases`.

Perf.haskell.org reports these nice performance wins:

    Nofib allocations
    fannkuch-redux    78446640  - 99.92%      64560
    k-nucleotide     109466384  - 91.32%    9502040
    simple            72424696  -  5.96%   68109560

    Nofib instruction counts
    fannkuch-redux  1744331636  -  3.86% 1676999519
    k-nucleotide    2318221965  -  6.30% 2172067260
    scs             1978470869  -  3.35% 1912263779
    simple           669858104  -  3.38%  647206739
    spectral-norm    186423292  -  5.37%  176411536

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

Summary:
#11721 changed the order of type variables in GADT
constructor type signatures, but these changes weren't reflected in
Template Haskell reification of GADTs. Let's do that.

Along the way, I:

* noticed that the `T13885` test was claiming to test TH reification
  of GADTs, but the reified data type wasn't actually a GADT! Since
  this patch touches that part of the codebase, I decided to fix
  this.
* incorporated some feedback from @simonpj in
  https://phabricator.haskell.org/D3687#113566. (These changes alone
  don't account for any different in behavior.)

Test Plan: make test TEST=T11721_TH

Reviewers: goldfire, austin, bgamari, simonpj

Reviewed By: goldfire, bgamari, simonpj

Subscribers: rwbarton, thomie, simonpj

GHC Trac Issues: #11721

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

After typechecking a data constructor's type signature, its type
variables are partitioned into two distinct groups: the universally
quantified type variables and the existentially quantified type
variables. Then, when prompted for the type of the data constructor,
GHC gives this:

```lang=haskell
MkT :: forall <univs> <exis>. (...)
```

For H98-style datatypes, this is a fine thing to do. But for GADTs,
this can sometimes produce undesired results with respect to
`TypeApplications`. For instance, consider this datatype:

```lang=haskell
data T a where
  MkT :: forall b a. b -> T a
```

Here, the user clearly intended to have `b` be available for visible
type application before `a`. That is, the user would expect
`MkT @Int @Char` to be of type `Int -> T Char`, //not//
`Char -> T Int`. But alas, up until now that was not how GHC
operated—regardless of the order in which the user actually wrote
the tyvars, GHC would give `MkT` the type:

```lang=haskell
MkT :: forall a b. b -> T a
```

Since `a` is universal and `b` is existential. This makes predicting
what order to use for `TypeApplications` quite annoying, as
demonstrated in #11721 and #13848.

This patch cures the problem by tracking more carefully the order in
which a user writes type variables in data constructor type
signatures, either explicitly (with a `forall`) or implicitly
(without a `forall`, in which case the order is inferred). This is
accomplished by adding a new field `dcUserTyVars` to `DataCon`, which
is a subset of `dcUnivTyVars` and `dcExTyVars` that is permuted to
the order in which the user wrote them. For more details, refer to
`Note [DataCon user type variables]` in `DataCon.hs`.

An interesting consequence of this design is that more data
constructors require wrappers. This is because the workers always
expect the first arguments to be the universal tyvars followed by the
existential tyvars, so when the user writes the tyvars in a different
order, a wrapper type is needed to swizzle the tyvars around to match
the order that the worker expects. For more details, refer to
`Note [Data con wrappers and GADT syntax]` in `MkId.hs`.

Test Plan: ./validate

Reviewers: austin, goldfire, bgamari, simonpj

Reviewed By: goldfire, simonpj

Subscribers: ezyang, goldfire, rwbarton, thomie

GHC Trac Issues: #11721, #13848

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

This dark corner was exposed by Trac #14285.  It involves the
interaction between absence analysis and INLINABLE pragmas.

There is a full explanation in Note [aBSENT_ERROR_ID] in MkCore,
which you can read there.  The changes in this patch are

* Make exprIsHNF return True for absentError, treating
  absentError like an honorary data constructor.

* Make absentError /not/ be diverging, unlike other error Ids.

This is all a bit horrible.

* While doing this I found that exprOkForSpeculation didn't
  have a case for value lambdas so I added one.  It's not
  really called on lifted types much, but it seems like the
  right thing

The check is a bit gnarly, but I couldn't think of a better way.
See the new code in TcTyClsDecls.

test case: polykinds/T13391

The strictness signature for a data con wrapper wasn't including any
dictionary arguments, which meant that bangs on the fields of a
constructor with an existential context would be moved to the wrong
fields.  See T14290 for an example.

Test Plan:
* New test T14290
* validate

Reviewers: simonpj, niteria, austin, bgamari, erikd

Reviewed By: simonpj, bgamari

Subscribers: rwbarton, thomie

GHC Trac Issues: #14290

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

This switches the compiler/ component to get compiled with
-XNoImplicitPrelude and a `import GhcPrelude` is inserted in all
modules.

This is motivated by the upcoming "Prelude" re-export of
`Semigroup((<>))` which would cause lots of name clashes in every
modulewhich imports also `Outputable`

Reviewers: austin, goldfire, bgamari, alanz, simonmar

Reviewed By: bgamari

Subscribers: goldfire, rwbarton, thomie, mpickering, bgamari

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

The demand signature of the retry# primop previously had a Diverges
result.  However, this caused the demand analyser to conclude that a
program of the shape,

    catchRetry# (... >> retry#)

would diverge. Of course, this is plainly wrong; catchRetry#'s sole
reason to exist is to "catch" the "exception" thrown by retry#. While
catchRetry#'s demand signature correctly had the ExnStr flag set on its
first argument, indicating that it should catch divergence, the logic
associated with this flag doesn't apply to Diverges results. This
resulted in #14171.

The solution here is to treat the divergence of retry# as an exception.
Namely, give it a result type of ThrowsExn rather than Diverges.

Updates stm submodule for tests.

Test Plan: Validate with T14171

Reviewers: simonpj, austin

Subscribers: rwbarton, thomie

GHC Trac Issues: #14171, #8091

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

the worker/wrapper creates an artificial INLINE pragma, which caused CSE
to not do its work. We now recognize such artificial pragmas by using
`NoUserInline` instead of `Inline` as the `InlineSpec`.

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

This is another take on https://phabricator.haskell.org/D3844.

This patch removes then need for haddock to reimplement the calculation
of exported names from modules. Instead when renaming export lists ghc
annotates each IE with its exported names.

Haddocks current  export logic has caused lots of trouble in the past
(on the Github issue tracker):
  - https://github.com/haskell/haddock/issues/121
  - https://github.com/haskell/haddock/issues/174
  - https://github.com/haskell/haddock/issues/225
  - https://github.com/haskell/haddock/issues/344
  - https://github.com/haskell/haddock/issues/584
  - https://github.com/haskell/haddock/issues/591
  - https://github.com/haskell/haddock/issues/597

Updates haddock submodule.

Reviewers: austin, bgamari, ezyang

Reviewed By: bgamari, ezyang

Subscribers: rwbarton, thomie

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

We pretty-print a type by converting it to an IfaceType and
pretty-printing that.  But
 (a) that's a bit indirect, and
 (b) delibrately loses information about (e.g.) the kind
      on the /occurrences/ of a type variable

So this patch implements debugPprType, which pretty prints
the type directly, with no fancy formatting.  It's just used
for debugging.

I took the opportunity to refactor the debug-pretty-printing
machinery a little.  In particular, define these functions
and use them:

  ifPprDeubug :: SDoc -> SDOc -> SDoc
    -- Says what to do with and without -dppr-debug
  whenPprDebug :: SDoc -> SDoc
    -- Says what to do with  -dppr-debug; without is empty
  getPprDebug :: (Bool -> SDoc) -> SDoc

getPprDebug used to be called sdocPprDebugWith
whenPprDebug used to be called ifPprDebug

So a lot of files get touched in a very mechanical way

No change in behaviour.

Provoked by Trac #14052

Summary:
Finish the work started in
7d1909ad.

Test Plan: If it builds, ship it

Reviewers: austin, bgamari

Subscribers: rwbarton, thomie

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

This was an old workaround for #5252. Fixes #13173.

Reviewers: austin, bgamari

Reviewed By: bgamari

Subscribers: rwbarton, thomie

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