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

Allow bidirectional typechecking in the plugin #105

Merged
merged 2 commits into from Jun 14, 2019

Conversation

isovector
Copy link
Member

@isovector isovector commented Jun 14, 2019

The plugin used to choke on this:

flipShouldBe 11 . sum . fst . run . runFoldMapOutput id $ do
  output [1]
  output $ replicate 2 5

because it would fail to unify Output (t Int) (the polymorphic Traversable variable in sum) with Output [Int].

In this case, there are no given constraints, so the plugin is attempting to solve a Member (Output (t Int)) '[Output [Int]]. We reuse the codepath for unifying wanted/givens, pretending like the
Output (t Int) is a given.1

So now we have a "wanted" Output [Int], and a "given" Output (t Int). In general, this thing isn't OK to solve. Consider a real example:

foo :: Member (Output (t Int)) r => Sem r ()
foo = output [5]

This is a type error, because the polymorphism goes the wrong way. We have no guarantees that t is supposed to be [].

But in our original example, this isn't a problem, because our t isn't actually in given position. It's just an artifact of reusing the code! mkWanted now takes a new parameter for whether or not it's OK to allow
the givens to be polymorphic.

Such a thing necessitates a change though. We never want to unify a polymorphic effect in given position. Doing so will break Haskell's regular type inference that determines what the effect row should be, based on the order in which the interpreters are run.

Fixes #95

Footnotes

  1. Pretending it's a wanted breaks the common case of trying to call, eg. runState (5 :: Num a => a) on an effect State Int.

The plugin used to choke on this:

```haskell
flipShouldBe 11 . sum . fst . run . runFoldMapOutput id $ do
  output [1]
  output $ replicate 2 5
```

because it would fail to unify `Output (t Int)` (the polymorphic
`Traversable` variable in `sum`) with `Output [Int]`.

In this case, there are no given constraints, so the plugin is
attempting to solve a `Member (Output (t Int)) '[Output [Int]]`. We
reuse the codepath for unifying wanted/givens, pretending like the
`Output (t Int)` is a given.[^1]

[^1]: Pretending it's a wanted breaks something else that I don't
remember right now. This code is brittle :(

So now we have a "wanted" `Output [Int]`, and a "given" `Output (t
Int)`. In general, this thing isn't OK to solve. Consider a real
example:

```haskell
foo :: Member (Output (t Int)) r => Sem r ()
foo = output [5]
```

This is a type error, because the polymorphism goes the wrong way. We
have no guarantees that `t` is supposed to be `[]`.

But in our original example, this isn't a problem, because our `t` isn't
actually in given position. It's just an artifact of reusing the code!
`mkWanted` now takes a new parameter for whether or not it's OK to allow
the givens to be polymorphic.

Such a thing necessitates a change though. We never want to unify
a polymorphic *effect* in given position. Doing so will break Haskell's
regular type inference that determines what the effect row should be,
based on the order in which the interpreters are run.

Fixes #95
@isovector
Copy link
Member Author

I've tested this against polysemy-zoo. I'm also trying to run it against hskit, but currently stuck in dependency hell. Will report back when I have the results.

@TheMatten
Copy link
Collaborator

If someone tries to put this into ghci (as I did 😄) - the reason it works here is that ghci has ExtendedDefaultRules enabled, which probably finds Foldable t from sum and fits [] in there.

Copy link
Collaborator

@TheMatten TheMatten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can extend comment about "given" constraint so that we don't forget what's going on here, but otherwise it seems fine to me 🙂

@isovector isovector merged commit e90c745 into master Jun 14, 2019
@isovector isovector deleted the plugin-poly-given-ok branch June 14, 2019 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin doesn't propagate unification information from inside to out
2 participants