serokell / universum

:milky_way: Prelude written in @Serokell
MIT License
176 stars 28 forks source link

[Chore] Update some versions #243

Closed gromakovsky closed 2 years ago

gromakovsky commented 3 years ago

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).

Related changes (conditional)

Stylistic guide (mandatory)

gromakovsky commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

See #249.