Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Selda bug on GHC 9.2.7 (with other changes from GHC 9.4.5 branch) #966

Closed
wants to merge 4 commits into from
Closed

Selda bug on GHC 9.2.7 (with other changes from GHC 9.4.5 branch) #966

wants to merge 4 commits into from

Conversation

dhess
Copy link
Member

@dhess dhess commented Apr 27, 2023 •
edited
Loading

Note to selves: this is a repro for the Selda bug that @georgefst found. To summarize:

brprice and others added 4 commits April 26, 2023 22:48
@brprice @dhess
chore: Upgrade to GHC 9.4.5.
5d24cb4 
Also, we do the following:

* Upgrade to fourmolu 0.12.0.0 and tell it not to put parens around
standalone unary constraints, for compatibility with hlint.

* Run a formatting pass to conform to fourmolu's new formatting.

* Build HLS from source. It doesn't currently build as a haskell.nix
tool with GHC 9.4.5, and we need to build it from git, anyway, in
order to get support for fourmolu 0.12.0.0.

* Disable weeder, which doesn't currently build as a haskell.nix tool
with GHC 9.4.5, and also fails to build on aarch64-darwin.

* Remove hie.yaml, as HLS once again seems able to operate without it.

Note: the definition of `type (<=)` changed in ghc-9.4 such that it
uses its arguments twice each. Thus `instance x <= y => C x y where
...` is rejected, since the constraint is a type synonym which expands
to something like `instance Assert (x <=? y) (LeErrMsg x y) => C x y
where` and this fails the Paterson Conditions:

https://downloads.haskell.org/ghc/9.4.4/docs/users_guide/exts/instances.html#instance-termination-rules

Using UndecidableInstances lifts these restrictions, satisfying GHC.
@georgefst @dhess
json fix
696d144
@dhess
Switch to GHC 9.2.7 to test the selda bug.
ebfbdba
@dhess
Undo a GHC 9.4.5-ism.
221c5db
@brprice
Copy link
Contributor

brprice commented Apr 27, 2023

more note to selves: the bug is that cabal run -O0 primer-selda-test (and -O2) seems to hang with ghc 9.2, but succeed with ghc 9.4 (pretty swiftly -- well under 20s). Before the "json fix" it succeeded with both.

When it hangs, it chews up a lot of cpu, but does not appear to leak memory

@brprice
Copy link
Contributor

brprice commented May 9, 2023

I'm fairly confident that this is an aeson/ghc bug: the part that chews cpu is in serialising a value to json (and it runs swiftly with either (a) not having OmitNothingFields or (b) "good" versions of ghc iirc these include 8.10.7, 9.0.* and 9.4.5, but not 9.2.* or 9.4.{1..4}). When I get some time I'll open an upstream issue (I have a very non-minimal repro showing the issue just depending on aeson) and link it here.

@brprice brprice mentioned this pull request May 25, 2023
Merged
@dhess
Copy link
Member Author

dhess commented Jun 18, 2023

I'm closing this, as it's not relevant to our repo anymore.

@dhess dhess closed this Jun 18, 2023
@dhess dhess deleted the dhess/ghc927-selda-test branch June 18, 2023 13:05
@dhess dhess added the wontfix No plans to address this label Jun 18, 2023
@brprice
Copy link
Contributor

brprice commented Jun 19, 2023

I'm fairly confident that this is an aeson/ghc bug: the part that chews cpu is in serialising a value to json (and it runs swiftly with either (a) not having OmitNothingFields or (b) "good" versions of ghc iirc these include 8.10.7, 9.0.* and 9.4.5, but not 9.2.* or 9.4.{1..4}). When I get some time I'll open an upstream issue (I have a very non-minimal repro showing the issue just depending on aeson) and link it here.

Upstream issue is at haskell/aeson#1030

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
wontfix No plans to address this
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dhess @brprice @georgefst