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.

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

[Chore] Update some versions #243

Merged
merged 7 commits into from
Nov 19, 2021
Merged

[Chore] Update some versions #243

merged 7 commits into from
Nov 19, 2021

Conversation

gromakovsky
Copy link
Member

Description

Just some casual maintenance.

Related issues(s)

None

✓ Checklist for your Pull Request

Ideally a PR has all of the checkmarks set.

If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).

  • I made sure my PR addresses a single concern, or multiple concerns which
    are inextricably linked. Otherwise I should open multiple PR's.
  • If your PR fixes/relates to an open issue then the description should
    reference this issue. See also auto linking on
    github
    .

Related changes (conditional)

  • Tests

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.
  • Documentation

    I checked whether I should update the docs and did so if necessary:

  • Record your changes

    • I added an entry to the changelog if my changes are visible to the users
      and
    • provided a migration guide for breaking changes if possible

Stylistic guide (mandatory)

  • My commit history is clean (only contains changes relating to my
    issue/pull request and no reverted-my-earlier-commit changes) and commit
    messages start with identifiers of related issues in square brackets.

    Example: [#42] Short commit description

    If necessary both of these can be achieved even after the commits have been
    made/pushed using rebase and squash.

Problem: lts-15.11 specified in stack.yaml is rather old.
Solution: update to the latest one.
Problem: in hashable-1.4 `Eq` is a superclass of `Hashable`,
it causes warnings in some code.

Solution: use CPP to check `hashable` version and replace
`(Eq a, Hashable a)` constraint with just `Hashable a` for new version.
Problem: it's hard to keep "Projects that use Universum" section
up-to-date. Now it mostly contains deprecated projects.

Solution: delete it since it's not really useful and rather confusing
(due to not being promptly updated).
Problem: IncoherentInstances extensions is deprecated,
it's recommended to use per-instance pragmas instead.

Solution: do not enable this extension, specify per-instance
pragmas where necessary.
Problem: for some reason ghc-9.0.1 is unable to compile benchmark code
which is successfully compiled by older versions of ghc.

Solution: provide explicit type annotations to help ghc compile this
code, it appeared to be sufficient.
@gromakovsky
Copy link
Member Author

@Martoon-00 AFAIU you are the author of VarArg.hs, so I'll be glad if you look at the last commit and check whether we need to care about it. I wonder if the usage on incoherent instances is fine, because apparently that's what caused that code to break. In particular, I wonder whether such breakages will always be solvable by simply specifying more explicit types. Probably it will be easier for you to understand what happened because you are familiar with that code and it's a bit hard to understand.

@Martoon-00
Copy link
Member

Martoon-00 commented Nov 18, 2021

That's true that ... became more fragile.

I believe this is a regression in GHC.

super :: [()] -> Bool
super = null ... asFun (: [])

asFun :: (a -> b) -> a -> b
asFun = id

This code works fine, while removing asFun breaks the code. So the instances resolver picks the wrong instance ("generic" one instead of "func" one) when no hint is supplied, thinking that (: []) is not of a -> type, but it actually is and GHC should've been able to see that.

While this breakage is sad and we have little guarantees on how INCOHERENT pragma should operate, ... worked well for quite a long time. I would create a ticket in GHC and see the reaction of the people there. If they treat it as a bug, then we can hope ... works for at least 5-10 year more after the fix; if they do not consider this an issue, then we probably should deprecate ....

Does it make sense?

Perhaps it would be nice to create a separate issue in this repository for tracking this, and assign it to someone to make sure there is someone responsible over it to have some progress over time.

@gromakovsky
Copy link
Member Author

I've asked about it in our ghc-general and without looking into the issue deeply the response was that it's not a bug in GHC. So now I want someone to look into the issue deeper and I chose you as the first person to ask because you were working on that code and probably it will take you less effort to think about it.

The way I understand it is that in general such a breakage may be not a compiler bug because {-# INCOHERENT #-} adds some non-determinism which can be handled differently by different compiler versions. Here I think the question is whether GHC is allowed to pick not the instance we want it to pick and whether this breakage actually happened due to this non-determinism. In order to understand it I think we need to carefully read about instance resolution and specifically about incoherent instances.

And also I am a bit worried about this:

I think GHC is choosing different instances now. Might even lead to different runtime behavior.

Anyway, you are right that we need a separate issue for it and I think it doesn't block this PR, so I'll proceed to merging it.

@gromakovsky
Copy link
Member Author

See #249.

@gromakovsky gromakovsky merged commit cade5a2 into master Nov 19, 2021
@delete-merged-branch delete-merged-branch bot deleted the gromak/chore-summer2021 branch November 19, 2021 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants