Massaging Upper Bounds #494

fosskers · 2019-05-03T16:45:57Z

This branch forks off from Pact right before the breaking changes of #482 . It tickles some of Pact's upper bounds, which allows for a simpler stack.yaml which full GHC 8.6 support, and eventually a simpler default.nix.

In general, this should probably not be merged. It could potentially be the basis of a 2.6.2 release, but we need not spend time on that now. Chainweb's own 8.6 support could be based off this branch in the short-term.

This branch also revealed some compilation issues, as explained in #495 .

Closes #495

Tested with `stack test` (all passing). Fixes #494.

fosskers · 2019-05-06T15:19:08Z

This passes tests locally (while on z3-4.8.3 of course)

sirlensalot

this looks good to me, and doesn't touch default.nix so I would think it's safe to merge.

fosskers · 2019-05-17T18:31:56Z

So long as we're okay with stack pointing to GHC 8.6, while Nix does not.

sirlensalot · 2019-05-17T20:53:28Z

@mightybyte any opinion here?

mightybyte · 2019-05-17T21:30:00Z

I don't have a problem with this. It could end up requiring more of our work to support both versions simultaneously, but as long as we're staying away from CPP territory it probably won't be a huge problem.

This allows the library to build (and be depended on) without "breaking open" the build with heavy-hammers like `--allow-newer` when compiling with GHC 8.6. The test suite still does not build, due to some naughty partial pattern matches.

These locations trigger errors on ghc 8.6 for missing `MonadFail` instances. Fixes #495.

Tested with `stack test` (all passing). Fixes #494.

fosskers · 2019-05-20T17:34:33Z

tests/RemoteVerifySpec.hs

+    ModuleData mod1 _refs <- ExceptT $ stateModuleData "mod1" replState0
+    ModuleData mod2 _refs <- ExceptT $ stateModuleData "mod2" replState0
+    ExceptT . fmap (first show) . serveAndRequest 3001 $
+      Remote.Request [derefDef <$> mod2, derefDef <$> mod1] "mod2"


An example of how to avoid partial pattern matches, as well as wanton error calls.

fosskers · 2019-05-20T21:00:54Z

.travis.yml


  # Linux build
  - env: BUILD=stack ARGS="-j1" Z3VERSION="4.8.3"
    addons:
      apt: {packages: [libgmp-dev]}
      artifacts:
        paths:
-          - .stack-work/dist/x86_64-linux/Cabal-2.2.0.1/build/pact/pact
+          - .stack-work/dist/x86_64-linux/Cabal-2.4.0.1/build/pact/pact


These paths would change with the bump to lts-13.*. Does another tool / integration expect these paths to have an exact shape (say 2.2 vs 2.4), say for deployment? I wouldn't want to break anything here.

Maybe windows? Trying to find out where our windows life is at currently elsewhere too. @vaibhavsagar ?

fosskers · 2019-05-20T21:08:45Z

This PR is now current with master.

@slpopejoy Unfortunately, the Analyze code still contains a number of instances of "running out the exhaustion checker". We currently have this in the updated stack.yaml:

# GHC 8.6 has a more aggressive Pattern Match exhaustion checker. A number of
# Pact's `Analysis` modules cause seeming infinite loops in the compiler during
# this analysis, so here we limit the depth to 5000, then override `-Werror`
# with `-Wwarn` to avoid failing the build.
ghc-options:
  pact: -fmax-pmcheck-iterations=5000 -Wwarn

This smooths things over for devs who use Stack during their day-to-day, but our Travis CI config reinforces -Werror, regardless of what's in stack.yaml or pact.cabal. So, these builds fail in Travis. The warnings themselves aren't strictly errors, and the pattern of code that leads to the warnings has always been there, it's just that GHC 8.6 in particular is smart enough to detect them, whereas 8.4 is not.

Thoughts about how this PR should proceed?

joelburget · 2019-05-20T22:51:09Z

@fosskers I can take a look at the Analyze problem in a few days when I get back from vacation

fosskers · 2019-05-21T03:32:44Z

Thanks @joelburget , no rush.

GHC 8.6's pattern match checker seems to have some bugs related to pattern synonyms (eg https://gitlab.haskell.org/ghc/ghc/issues/14253). We remove a few places where the `SObject` pattern synonym erroneously triggered a failure. We can also remove the `-fmax-pmcheck-iterations=5000` option. Tested via: ``` cabal clean; cabal test stack clean; stack test ```

joelburget · 2019-05-24T17:34:18Z

I was able to remove the -fmax-pmcheck-iterations options by removing some pattern synonyms. Do let me know if there are more analysis issues.

fosskers · 2019-05-24T18:16:48Z

Thanks @joelburget , testing now.

sirlensalot

Approved. We might need to figure out how this impacts the Windows build but since we're delayed getting an answer would be good to get this in.

fosskers mentioned this pull request May 3, 2019

Consider avoiding partial pattern matches in tests #495

Closed

joelburget added a commit that referenced this pull request May 3, 2019

Fix remaining monad pattern matches for ghc 8.6.

787b465

Tested with `stack test` (all passing). Fixes #494.

joelburget force-pushed the colin/ghc86-massaging branch from 57b6250 to e52725e Compare May 6, 2019 17:04

sirlensalot reviewed May 17, 2019

View reviewed changes

fosskers and others added 6 commits May 20, 2019 08:55

Massaging upper bounds

9f07b62

This allows the library to build (and be depended on) without "breaking open" the build with heavy-hammers like `--allow-newer` when compiling with GHC 8.6. The test suite still does not build, due to some naughty partial pattern matches.

Remove partial pattern matches from hedgehog generators.

e9ec39f

These locations trigger errors on ghc 8.6 for missing `MonadFail` instances. Fixes #495.

Fix remaining monad pattern matches for ghc 8.6.

27c4cc7

Tested with `stack test` (all passing). Fixes #494.

[stack] Bump sbv pin [skip ci]

3a8934f

[stack] Bump resolver

cef5d9c

Fix compile errors

baa5fe1

fosskers force-pushed the colin/ghc86-massaging branch from e52725e to baa5fe1 Compare May 20, 2019 17:29

fosskers commented May 20, 2019

View reviewed changes

[travis] Modify artefact paths

1852358

fosskers commented May 20, 2019

View reviewed changes

Remove unneeded sections from cabal.project

cbcb26c

fosskers changed the title ~~[side branch] Massaging Upper Bounds~~ Massaging Upper Bounds May 24, 2019

sirlensalot approved these changes May 24, 2019

View reviewed changes

sirlensalot merged commit e1889e9 into master May 24, 2019

sirlensalot deleted the colin/ghc86-massaging branch May 24, 2019 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Massaging Upper Bounds #494

Massaging Upper Bounds #494

fosskers commented May 3, 2019 •

edited

Loading

fosskers commented May 6, 2019 •

edited

Loading

sirlensalot left a comment

fosskers commented May 17, 2019

sirlensalot commented May 17, 2019

mightybyte commented May 17, 2019

fosskers May 20, 2019

fosskers May 20, 2019

sirlensalot May 24, 2019

fosskers commented May 20, 2019 •

edited

Loading

joelburget commented May 20, 2019

fosskers commented May 21, 2019

joelburget commented May 24, 2019

fosskers commented May 24, 2019

sirlensalot left a comment

Massaging Upper Bounds #494

Massaging Upper Bounds #494

Conversation

fosskers commented May 3, 2019 • edited Loading

fosskers commented May 6, 2019 • edited Loading

sirlensalot left a comment

Choose a reason for hiding this comment

fosskers commented May 17, 2019

sirlensalot commented May 17, 2019

mightybyte commented May 17, 2019

fosskers May 20, 2019

Choose a reason for hiding this comment

fosskers May 20, 2019

Choose a reason for hiding this comment

sirlensalot May 24, 2019

Choose a reason for hiding this comment

fosskers commented May 20, 2019 • edited Loading

joelburget commented May 20, 2019

fosskers commented May 21, 2019

joelburget commented May 24, 2019

fosskers commented May 24, 2019

sirlensalot left a comment

Choose a reason for hiding this comment

fosskers commented May 3, 2019 •

edited

Loading

fosskers commented May 6, 2019 •

edited

Loading

fosskers commented May 20, 2019 •

edited

Loading