GitLab

We want to remove the `Ord Unique` instance because there's
no way to implement it in deterministic way and it's too
easy to use by accident.

We sometimes compute SCC for datatypes whose Ord instance
is implemented in terms of Unique. The Ord constraint on
SCC is just an artifact of some internal data structures.
We can have an alternative implementation with a data
structure that uses Uniquable instead.

This does exactly that and I'm pleased that I didn't have
to introduce any duplication to do that.

Test Plan:
./validate
I looked at performance tests and it's a tiny bit better.

Reviewers: bgamari, simonmar, ezyang, austin, goldfire

Subscribers: thomie

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

GHC Trac Issues: #4012

Summary:
This patch uses collectArgsTicks instead of collectArgs to test that
StaticPtr only occurs at the top of RHSs of top-level expressions.

Ticks introduced by -fhpc would interfere otherwise.

Test Plan: ./validate

Reviewers: thomie, austin, goldfire, bgamari, simonpj

Reviewed By: simonpj

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

GHC Trac Issues: #12207

In reviewing the fix to Trac #12130 I found the wild-card
fill-in code for ".." notation in record constructions hard
to understand.  It went to great contortions (including the
find_tycon code) to allow
    data T = C { x, y :: Int }
    f x = C { .. }
to expand to
    f x = C { x = x, y = y }
where 'y' is an /imported function/!  That seems way over the top
for what record wildcards are supposed to do.

So I have narrowed the record-wildcard expansion to include only
/locally-bound/ variables; i.e. not top level, and certainly not
imported.

I don't think anyone is using record wildcards in this bizarre way, so
I don't expect any fallout. Even if there is, you can easily
initialise fields with eponymous but imported values by hand.

An intermediate position would be to allow /local/ top-level
definitions.  But I doubt anyone is doing that either.

Let's see if there's any fallout.  It's a local change, easy to
revert, so I've just gone ahead to save everyone's time.

In Trac #11948 I added the warning
    -Wsimplifiable-class-constraints
which warns if the class constraints in a type signature are
simplifiable.

But in fact the fragility it warns about only happens with
NoMonoLocalBinds, so this patch switches off the warning if
you have MonoLocalBinds (and suggests using it in the error
message).

See Note [Simplifiable given constraints] in TcValidity.

lookupGRE_Name should return either zero or one GREs, never
several. This is a consequence of INVARIANT 1 on GlobalRdrEnv.

So it's better if it returns a Maybe; the panic on multiple results
is put in one place, instead of being scattered or ignored.

Just refactoring, no change in behaviour

This patch fixes Trac #12175, another delicate corner case of
Note [Instance and Given overlap] in TcInteract.

In #12175 we were not expanding given superclasses eagerly
enough. It was easy to fix, and is actually rather neater than
before.

See Note [Eagerly expand given superclasses] in TcCanonical.
The main change is to move the eager expansion of given superclasses
to canClassNC.

We don't need the FamInstEnvs argument any more.
Just a tiny refactor.

Fixes Trac #12163.  Pretty simple.

The `Ord Module` instance currently uses `Unique`s for comparison.
We don't want to use the `Unique` order because it can introduce
nondeterminism.
This switches `Ord ModuleName` and `Ord UnitId` to use lexicographic
ordering making `Ord Module` deterministic transitively.

I've run `nofib` and it doesn't make a measurable difference.

See also Note [ModuleEnv determinism and performance].

This fixes #12191 - the regression, that the previous version of this
patch had.

Test Plan:
./validate
run nofib: P112

Reviewers: simonmar, bgamari, austin

Subscribers: thomie

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

GHC Trac Issues: #4012, #12191

The warning for reference:
```
rts/RaiseAsync.c: In function ‘throwToMsg’:

rts/SMPClosureOps.h:65:0: error:
     error: inlining failed in call to ‘lockClosure’: call is unlikely
and code size would grow

rts/RaiseAsync.c:305:0: error:  error: called from here

rts/SMPClosureOps.h:65:0: error:
     error: inlining failed in call to ‘lockClosure’: call is unlikely
and code size would grow
```

This warning triggers on `gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-16)`
and it doesn't trigger with new GCCs.

Test Plan: build ghc/rts

Reviewers: bgamari, simonmar, austin

Subscribers: thomie

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

Summary:
Background:
* sizeExpr was calculating expressions like ((e `cast` T) x) wrongly
* Fixing it caused regressions in compile performance, and one nofib
  program (k-nucleotide)

I managed to fix the source of the compiler regressions.  I think it was
due to traceTc not being inlined, which I fixed in a more robust way by
putting an export list on TcRnMonad.

The k-nucleotide regression is more difficult.  I don't think anything
is actually going wrong, but this program has been highly tuned and is
quite sensitive to changing in inlining behaviour.  I managed to recover
most of the performance by manual lambda-lifting which makes it a bit
less fragile, but the end result was a bit slower.  I don't think this
is disastrous, the program is pretty horrible to begin with and we could
probably make a faster one by starting from scratch.

Test Plan: validate, nofib

Reviewers: simonpj, bgamari, niteria, austin, erikd

Subscribers: thomie

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

GHC Trac Issues: #11564

Doing canonicalization on every comparison turned
out to be very expensive.

Caching the canonicalization through the smart `modulePair` constructor
gives `8%` reduction in allocations on `haddock.compiler` and
`8.5%` reduction in allocations on `haddock.Cabal`.
Possibly other things as well, but it's really visible in Haddock.

Test Plan: ./validate

Reviewers: jstolarek, simonpj, austin, simonmar, bgamari

Reviewed By: simonpj, simonmar

Subscribers: thomie

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

GHC Trac Issues: #12191

Richard: in a previous commit I combined the two case for

   decideQuantification

This commit just deletes the old code. I'm afraid it'll leave you
with a merge conflict though, with your stuff on generalisation.

This fixes Trac #12212.  It's quite hard to provoke, but I've
added a standalone test case that does so.

The issue is explained in Note [Evidence foralls] in Specialise.

Merge the following functions into one:
  * rawSystem
  * rawSystemWithTimeout
  * runCmd
  * runCmdFor
  * runCmdExitCode

I don't know why this wasn't done before.

* Set config settings directly in mk/test.mk, instead of indirectly in
  config/ghc
* passing --hpcdir for WAY=hpc is unnecessary

This allows the removal of the override_flags stuff in testlib.py.

The previous commits removed `-fforce-recomp` from TEST_HC_OPTS, so
TEST_HC_OPTS_NO_RECOMP = TEST_HC_OPTS.

There is no need for this flag anymore, since each test runs in a
newly created directory. Removing it cleans up testlib.py a bit.

There is a small risk that this renders some tests useless. It's hard to
know. Those tests should have specified -fforce-recomp` explicitly
anyway, so I'm not going to worry about it. I've fixed the ones that
failed without -fforce-recomp.

Reviewed by: bgamari

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

* CgStaticPointers, GcStaticPointers, ListStaticPointers,
  TcStaticPointers01, TcStaticPointers02:  #12207
* T11535: #12210
* ffi017/ffi021: #12209
* T11108: #11108
* T9646: #9646

See `Note [Replacing backward slashes in config.libdir]`

There is one caveat: in ae4acbd1
I mentioned:

  > Changing backwards slashes to forward slashes apparently confuses
  > msys2/mingw magic path handling.

I can not reproduce that problem anymore, however.

This patch validates for me, and fixes all tests that use config.libdir
for WAY=ghci. We'll see how it goes.