1. 26 Sep, 2020 4 commits
  2. 24 Sep, 2020 3 commits
    • Sebastian Graf's avatar
      PmCheck: Desugar string literal patterns with -XRebindableSyntax correctly (#18708) · 6d0ce0eb
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      Fixes #18708.
      6d0ce0eb
    • Simon Peyton Jones's avatar
      Improve kind generalisation, error messages · 9fa26aa1
      Simon Peyton Jones authored and  Marge Bot's avatar Marge Bot committed
      This patch does two things:
      
      * It refactors GHC.Tc.Errors a bit.  In debugging Quick Look I was
        forced to look in detail at error messages, and ended up doing a bit
        of refactoring, esp in mkTyVarEqErr'.  It's still quite a mess, but
        a bit better, I think.
      
      * It makes a significant improvement to the kind checking of type and
        class declarations. Specifically, we now ensure that if kind
        checking fails with an unsolved constraint, all the skolems are in
        scope.  That wasn't the case before, which led to some obscure error
        messages; and occasional failures with "no skolem info" (eg #16245).
      
      Both of these, and the main Quick Look patch itself, affect a /lot/ of
      error messages, as you can see from the number of files changed.  I've
      checked them all; I think they are as good or better than before.
      
      Smaller things
      
      * I documented the various instances of VarBndr better.
        See Note [The VarBndr tyep and its uses] in GHC.Types.Var
      
      * Renamed GHC.Tc.Solver.simpl_top to simplifyTopWanteds
      
      * A bit of refactoring in bindExplicitTKTele, to avoid the
        footwork with Either.  Simpler now.
      
      * Move promoteTyVar from GHC.Tc.Solver to GHC.Tc.Utils.TcMType
      
      Fixes #16245 (comment 211369), memorialised as
        typecheck/polykinds/T16245a
      Also fixes the three bugs in #18640
      9fa26aa1
    • Simon Peyton Jones's avatar
      Implement Quick Look impredicativity · 97cff919
      Simon Peyton Jones authored and  Marge Bot's avatar Marge Bot committed
      This patch implements Quick Look impredicativity (#18126), sticking
      very closely to the design in
          A quick look at impredicativity, Serrano et al, ICFP 2020
      
      The main change is that a big chunk of GHC.Tc.Gen.Expr has been
      extracted to two new modules
          GHC.Tc.Gen.App
          GHC.Tc.Gen.Head
      which deal with typechecking n-ary applications, and the head of
      such applications, respectively.  Both contain a good deal of
      documentation.
      
      Three other loosely-related changes are in this patch:
      
      * I implemented (partly by accident) points (2,3)) of the accepted GHC
        proposal "Clean up printing of foralls", namely
        https://github.com/ghc-proposals/ghc-proposals/blob/
              master/proposals/0179-printing-foralls.rst
        (see #16320).
      
        In particular, see Note [TcRnExprMode] in GHC.Tc.Module
        - :type instantiates /inferred/, but not /specified/, quantifiers
        - :type +d instantiates /all/ quantifiers
        - :type +v is killed off
      
        That completes the implementation of the proposal,
        since point (1) was done in
          commit df084681
          Author: Krzysztof Gogolewski <krzysztof.gogolewski@tweag.io>
          Date:   Mon Feb 3 21:17:11 2020 +0100
          Always display inferred variables using braces
      
      * HsRecFld (which the renamer introduces for record field selectors),
        is now preserved by the typechecker, rather than being rewritten
        back to HsVar.  This is more uniform, and turned out to be more
        convenient in the new scheme of things.
      
      * The GHCi debugger uses a non-standard unification that allows the
        unification variables to unify with polytypes.  We used to hack
        this by using ImpredicativeTypes, but that doesn't work anymore
        so I introduces RuntimeUnkTv.  See Note [RuntimeUnkTv] in
        GHC.Runtime.Heap.Inspect
      
      Updates haddock submodule.
      
      WARNING: this patch won't validate on its own.  It was too
      hard to fully disentangle it from the following patch, on
      type errors and kind generalisation.
      
      Changes to tests
      
      * Fixes #9730 (test added)
      
      * Fixes #7026 (test added)
      
      * Fixes most of #8808, except function `g2'` which uses a
        section (which doesn't play with QL yet -- see #18126)
        Test added
      
      * Fixes #1330. NB Church1.hs subsumes Church2.hs, which is now deleted
      
      * Fixes #17332 (test added)
      
      * Fixes #4295
      
      * This patch makes typecheck/should_run/T7861 fail.
        But that turns out to be a pre-existing bug: #18467.
        So I have just made T7861 into expect_broken(18467)
      97cff919
  3. 22 Sep, 2020 3 commits
    • Sebastian Graf's avatar
      PmCheck: Rewrite inhabitation test · e9501547
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      We used to produce inhabitants of a pattern-match refinement type Nabla
      in the checker in at least two different and mostly redundant ways:
      
        1. There was `provideEvidence` (now called
           `generateInhabitingPatterns`) which is used by
           `GHC.HsToCore.PmCheck` to produce non-exhaustive patterns, which
           produces inhabitants of a Nabla as a sub-refinement type where all
           match variables are instantiated.
        2. There also was `ensure{,All}Inhabited` (now called
           `inhabitationTest`) which worked slightly different, but was
           whenever new type constraints or negative term constraints were
           added. See below why `provideEvidence` and `ensureAllInhabited`
           can't be the same function, the main reason being performance.
        3. And last but not least there was the `nonVoid` test, which tested
           that a given type was inhabited. We did use this for strict fields
           and -XEmptyCase in the past.
      
      The overlap of (3) with (2) was always a major pet peeve of mine. The
      latter was quite efficient and proven to work for recursive data types,
      etc, but could not handle negative constraints well (e.g. we often want
      to know if a *refined* type is empty, such as `{ x:[a] | x /= [] }`).
      
      Lower Your Guards suggested that we could get by with just one, by
      replacing both functions with `inhabitationTest` in this patch.
      That was only possible by implementing the structure of φ constraints
      as in the paper, namely the semantics of φ constructor constraints.
      
      This has a number of benefits:
      
        a. Proper handling of unlifted types and strict fields, fixing #18249,
           without any code duplication between
           `GHC.HsToCore.PmCheck.Oracle.instCon` (was `mkOneConFull`) and
           `GHC.HsToCore.PmCheck.checkGrd`.
        b. `instCon` can perform the `nonVoid` test (3) simply by emitting
           unliftedness constraints for strict fields.
        c. `nonVoid` (3) is thus simply expressed by a call to
           `inhabitationTest`.
        d. Similarly, `ensureAllInhabited` (2), which we called after adding
           type info, now can similarly be expressed as the fuel-based
           `inhabitationTest`.
      
      See the new `Note [Why inhabitationTest doesn't call generateInhabitingPatterns]`
      why we still have tests (1) and (2).
      
      Fixes #18249 and brings nice metric decreases for `T17836` (-76%) and
      `T17836b` (-46%), as well as `T18478` (-8%) at the cost of a few very
      minor regressions (< +2%), potentially due to the fact that
      `generateInhabitingPatterns` does more work to suggest the minimal
      COMPLETE set.
      
      Metric Decrease:
          T17836
          T17836b
      e9501547
    • Simon Peyton Jones's avatar
      Fix the occurrence analyser · 416bd50e
      Simon Peyton Jones authored and  Marge Bot's avatar Marge Bot committed
      Ticket #18603 demonstrated that the occurrence analyser's
      handling of
      
        local RULES for imported Ids
      
      (which I now call IMP-RULES) was inadequate.  It led the simplifier
      into an infnite loop by failing to label a binder as a loop breaker.
      
      The main change in this commit is to treat IMP-RULES in a simple and
      uniform way: as extra rules for the local binder.  See
        Note [IMP-RULES: local rules for imported functions]
      
      This led to quite a bit of refactoring.  The result is still tricky,
      but it's much better than before, and better documented I think.
      
      Oh, and it fixes the bug.
      416bd50e
    • Simon Peyton Jones's avatar
      Better eta-expansion (again) and don't specilise DFuns · 6de40f83
      Simon Peyton Jones authored and  Marge Bot's avatar Marge Bot committed
      This patch fixes #18223, which made GHC generate an exponential
      amount of code.  There are three quite separate changes in here
      
      1.  Re-engineer eta-expansion (again).  The eta-expander was
          generating lots of intermediate stuff, which could be optimised
          away, but which choked the simplifier meanwhile.  Relatively
          easy to kill it off at source.
      
          See Note [The EtaInfo mechanism] in GHC.Core.Opt.Arity.
          The main new thing is the use of pushCoArg in getArg_maybe.
      
      2.  Stop Specialise specalising DFuns.  This is the cause of a huge
          (and utterly unnecessary) blowup in program size in #18223.
          See Note [Do not specialise DFuns] in GHC.Core.Opt.Specialise.
      
          I also refactored the Specialise monad a bit... it was silly,
          because it passed on unchanging values as if they were mutable
          state.
      
      3.  Do an extra Simplifer run, after SpecConstra and before
          late-Specialise.  I found (investigating perf/compiler/T16473)
          that failing to do this was crippling *both* SpecConstr *and*
          Specialise.  See Note [Simplify after SpecConstr] in
          GHC.Core.Opt.Pipeline.
      
          This change does mean an extra run of the Simplifier, but only
          with -O2, and I think that's acceptable.
      
          T16473 allocates *three* times less with this change.  (I changed
          it to check runtime rather than compile time.)
      
      Some smaller consequences
      
      * I moved pushCoercion, pushCoArg and friends from SimpleOpt
        to Arity, because it was needed by the new etaInfoApp.
      
        And pushCoValArg now returns a MCoercion rather than Coercion for
        the argument Coercion.
      
      * A minor, incidental improvement to Core pretty-printing
      
      This does fix #18223, (which was otherwise uncompilable. Hooray.  But
      there is still a big intermediate because there are some very deeply
      nested types in that program.
      
      Modest reductions in compile-time allocation on a couple of benchmarks
          T12425     -2.0%
          T13253    -10.3%
      
      Metric increase with -O2, due to extra simplifier run
          T9233     +5.8%
          T12227    +1.8%
          T15630    +5.0%
      
      There is a spurious apparent increase on heap residency on T9630,
      on some architectures at least.  I tried it with -G1 and the residency
      is essentially unchanged.
      
      Metric Increase
          T9233
          T12227
          T9630
      
      Metric Decrease
          T12425
          T13253
      6de40f83
  4. 21 Sep, 2020 7 commits
  5. 19 Sep, 2020 1 commit
  6. 17 Sep, 2020 2 commits
  7. 15 Sep, 2020 2 commits
    • Sylvain Henry's avatar
      Enhance metrics output · b3143f5a
      Sylvain Henry authored and  Marge Bot's avatar Marge Bot committed
      b3143f5a
    • Simon Peyton Jones's avatar
      Care with implicit-parameter superclasses · c7182a5c
      Simon Peyton Jones authored and  Marge Bot's avatar Marge Bot committed
      Two bugs, #18627 and #18649, had the same cause: we were not
      account for the fact that a constaint tuple might hide an implicit
      parameter.
      
      The solution is not hard: look for implicit parameters in
      superclasses.  See Note [Local implicit parameters] in
      GHC.Core.Predicate.
      
      Then we use this new function in two places
      
      * The "short-cut solver" in GHC.Tc.Solver.Interact.shortCutSolver
        which simply didn't handle implicit parameters properly at all.
        This fixes #18627
      
      * The specialiser, which should not specialise on implicit parameters
        This fixes #18649
      
      There are some lingering worries (see Note [Local implicit
      parameters]) but things are much better.
      c7182a5c
  8. 13 Sep, 2020 1 commit
    • Sebastian Graf's avatar
      Make `tcCheckSatisfiability` incremental (#18645) · 69ea2fee
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      By taking and returning an `InertSet`.
      Every new `TcS` session can then pick up where a prior session left with
      `setTcSInerts`.
      
      Since we don't want to unflatten the Givens (and because it leads to
      infinite loops, see !3971), we introduced a new variant of `runTcS`,
      `runTcSInerts`, that takes and returns the `InertSet` and makes
      sure not to unflatten the Givens after running the `TcS` action.
      
      Fixes #18645 and #17836.
      
      Metric Decrease:
          T17977
          T18478
      69ea2fee
  9. 12 Sep, 2020 2 commits
    • Krzysztof Gogolewski's avatar
      Make sure we can read past perf notes · 8440b5fa
      Krzysztof Gogolewski authored and  Marge Bot's avatar Marge Bot committed
      See #18656.
      8440b5fa
    • Sebastian Graf's avatar
      PmCheck: Disattach COMPLETE pragma lookup from TyCons · 2a942285
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      By not attaching COMPLETE pragmas with a particular TyCon and instead
      assume that every COMPLETE pragma is applicable everywhere, we can
      drastically simplify the logic that tries to initialise available
      COMPLETE sets of a variable during the pattern-match checking process,
      as well as fixing a few bugs.
      
      Of course, we have to make sure not to report any of the
      ill-typed/unrelated COMPLETE sets, which came up in a few regression
      tests.
      
      In doing so, we fix #17207, #18277 and #14422.
      
      There was a metric decrease in #18478 by ~20%.
      
      Metric Decrease:
          T18478
      2a942285
  10. 10 Sep, 2020 3 commits
    • Sebastian Graf's avatar
      PmCheck: Handle ⊥ and strict fields correctly (#18341) · 3777be14
      Sebastian Graf authored
      In #18341, we discovered an incorrect digression from Lower Your Guards.
      This MR changes what's necessary to support properly fixing #18341.
      
      In particular, bottomness constraints are now properly tracked in the
      oracle/inhabitation testing, as an additional field
      `vi_bot :: Maybe Bool` in `VarInfo`. That in turn allows us to
      model newtypes as advertised in the Appendix of LYG and fix #17725.
      Proper handling of ⊥ also fixes #17977 (once again) and fixes #18670.
      
      For some reason I couldn't follow, this also fixes #18273.
      
      I also added a couple of regression tests that were missing. Most of
      them were already fixed before.
      
      In summary, this patch fixes #18341, #17725, #18273, #17977 and #18670.
      
      Metric Decrease:
          T12227
      3777be14
    • Sebastian Graf's avatar
      PmCheck: Big refactor using guard tree variants more closely following source syntax (#18565) · 1207576a
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      Previously, we desugared and coverage checked plain guard trees as
      described in Lower Your Guards. That caused (in !3849) quite a bit of
      pain when we need to partially recover tree structure of the input
      syntax to return covered sets for long-distance information, for
      example.
      
      In this refactor, I introduced a guard tree variant for each relevant
      source syntax component of a pattern-match (mainly match groups, match,
      GRHS, empty case, pattern binding). I made sure to share as much
      coverage checking code as possible, so that the syntax-specific checking
      functions are just wrappers around the more substantial checking
      functions for the LYG primitives (`checkSequence`, `checkGrds`).
      
      The refactoring payed off in clearer code and elimination of all panics
      related to assumed guard tree structure and thus fixes #18565.
      
      I also took the liberty to rename and re-arrange the order of functions
      and comments in the module, deleted some dead and irrelevant Notes,
      wrote some new ones and gave an overview module haddock.
      1207576a
    • Sebastian Graf's avatar
      Add long-distance info for pattern bindings (#18572) · 67ce72da
      Sebastian Graf authored and  Marge Bot's avatar Marge Bot committed
      We didn't consider the RHS of a pattern-binding before, which led to
      surprising warnings listed in #18572.
      
      As can be seen from the regression test T18572, we get the expected
      output now.
      67ce72da
  11. 09 Sep, 2020 4 commits
    • Ben Gamari's avatar
      gitlab-ci: Bump Docker images · 5aae5b32
      Ben Gamari authored and  Marge Bot's avatar Marge Bot committed
      We now generate our Docker images via Dhall definitions, as described in
      ghc/ci-images!52. Additionally, we are far more careful about where tools
      come from, using the ALEX, HAPPY, HSCOLOR, and GHC environment variables
      (set in the Dockerfiles) to find bootstrapping tools.
      5aae5b32
    • Alan Zimmerman's avatar
      Remove GENERATED pragma, as it is not being used · 7911d0d9
      Alan Zimmerman authored and  Marge Bot's avatar Marge Bot committed
      @alanz pointed out on ghc-devs that the payload of this pragma does
      not appear to be used anywhere.
      
      I (@bgamari) did some digging and traced the pragma's addition back to
      d386e0d2 (way back in 2006!).
      
      It appears that it was intended to be used by code generators for use
      in informing the code coveraging checker about generated code
      provenance. When it was added it used the pragma's "payload" fields as
      source location information to build an "ExternalBox". However, it
      looks like this was dropped a year later in 55a5d8d9.  At this point
      it seems like the pragma serves no useful purpose.
      
      Given that it also is not documented, I think we should remove it.
      
      Updates haddock submodule
      
      Closes #18639
      7911d0d9
    • Sylvain Henry's avatar
      DynFlags: add UnfoldingOpts and SimpleOpts · 3f32a9c0
      Sylvain Henry authored and  Marge Bot's avatar Marge Bot committed
      Milestone: after this patch, we only use 'unsafeGlobalDynFlags' for the
      state hack and for debug in Outputable.
      3f32a9c0
    • Ryan Scott's avatar
      Postpone associated tyfam default checks until after typechecking · 822f1057
      Ryan Scott authored and  Marge Bot's avatar Marge Bot committed
      Previously, associated type family defaults were validity-checked
      during typechecking. Unfortunately, the error messages that these
      checks produce run the risk of printing knot-tied type constructors,
      which will cause GHC to diverge. In order to preserve the current
      error message's descriptiveness, this patch postpones these validity
      checks until after typechecking, which are now located in the new
      function `GHC.Tc.Validity.checkValidAssocTyFamDeflt`.
      
      Fixes #18648.
      822f1057
  12. 08 Sep, 2020 3 commits
    • Ryan Scott's avatar
      Make the forall-or-nothing rule only apply to invisible foralls (#18660) · 44472daf
      Ryan Scott authored and  Marge Bot's avatar Marge Bot committed
      This fixes #18660 by changing `isLHsForAllTy` to
      `isLHsInvisForAllTy`, which is sufficient to make the
      `forall`-or-nothing rule only apply to invisible `forall`s. I also
      updated some related documentation and Notes while I was in the
      neighborhood.
      44472daf
    • Daishi Nakajima's avatar
      testsuite: Output performance test results in tabular format · d7b2f799
      Daishi Nakajima authored and  Marge Bot's avatar Marge Bot committed
      this was suggested in #18417.
      
      Change the print format of the values.
      * Shorten commit hash
      * Reduce precision of the "Value" field
      * Shorten metrics name
        * e.g. runtime/bytes allocated -> run/alloc
      * Shorten "MetricsChange"
        * e.g. unchanged -> unch, increased -> incr
      
      And, print the baseline environment if there are baselines that were
      measured in a different environment than the current environment.
      
      If all "Baseline commit" are the same, print it once.
      d7b2f799
    • Moritz Angermann's avatar
      [macOS] improved runpath handling · 4ff93292
      Moritz Angermann authored and  Marge Bot's avatar Marge Bot committed
      In b592bd98 we started using
      -dead_strip_dylib on macOS when lining dynamic libraries and binaries.
      The underlying reason being the Load Command Size Limit in macOS
      Sierra (10.14) and later.
      
      GHC will produce @rpath/libHS... dependency entries together with a
      corresponding RPATH entry pointing to the location of the libHS...
      library. Thus for every library we produce two Load Commands.  One to
      specify the dependent library, and one with the path where to find it.
      This makes relocating libraries and binaries easier, as we just need to
      update the RPATH entry with the install_name_tool. The dynamic linker
      will then subsitute each @rpath with the RPATH entries it finds in the
      libraries load commands or the environement, when looking up @rpath
      relative libraries.
      
      -dead_strip_dylibs intructs the linker to drop unused libraries. This in
      turn help us reduce the number of referenced libraries, and subsequently
      the size of the load commands.  This however does not remove the RPATH
      entries.  Subsequently we can end up (in extreme cases) with only a
      single @rpath/libHS... entry, but 100s or more RPATH entries in the Load
      Commands.
      
      This patch rectifies this (slighly unorthodox) by passing *no* -rpath
      arguments to the linker at link time, but -headerpad 8000.  The
      headerpad argument is in hexadecimal and the maxium 32k of the load
      command size.  This tells the linker to pad the load command section
      enough for us to inject the RPATHs later.  We then proceed to link the
      library or binary with -dead_strip_dylibs, and *after* the linking
      inspect the library to find the left over (non-dead-stripped)
      dependencies (using otool).  We find the corresponding RPATHs for each
      @rpath relative dependency, and inject them into the library or binary
      using the install_name_tool.  Thus achieving a deadstripped dylib (and
      rpaths) build product.
      
      We can not do this in GHC, without starting to reimplement a dynamic
      linker as we do not know which symbols and subsequently libraries are
      necessary.
      
      Commissioned-by: Mercury Technologies, Inc. (mercury.com)
      4ff93292
  13. 05 Sep, 2020 2 commits
  14. 04 Sep, 2020 1 commit
    • Ryan Scott's avatar
      Introduce isBoxedTupleDataCon and use it to fix #18644 · c1e54439
      Ryan Scott authored and  Marge Bot's avatar Marge Bot committed
      The code that converts promoted tuple data constructors to
      `IfaceType`s in `GHC.CoreToIface` was using `isTupleDataCon`, which
      conflates boxed and unboxed tuple data constructors. To avoid this,
      this patch introduces `isBoxedTupleDataCon`, which is like
      `isTupleDataCon` but only works for _boxed_ tuple data constructors.
      
      While I was in town, I was horribly confused by the fact that there
      were separate functions named `isUnboxedTupleCon` and
      `isUnboxedTupleTyCon` (similarly, `isUnboxedSumCon` and
      `isUnboxedSumTyCon`). It turns out that the former only works for
      data constructors, despite its very general name! I opted to rename
      `isUnboxedTupleCon` to `isUnboxedTupleDataCon` (similarly, I renamed
      `isUnboxedSumCon` to `isUnboxedSumDataCon`) to avoid this potential
      confusion, as well as to be more consistent with
      the naming convention I used for `isBoxedTupleDataCon`.
      
      Fixes #18644.
      c1e54439
  15. 01 Sep, 2020 2 commits