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

Incomplete inferring for if expressions? #9

Closed
FrankBro opened this issue Aug 23, 2018 · 4 comments
Closed

Incomplete inferring for if expressions? #9

FrankBro opened this issue Aug 23, 2018 · 4 comments
Assignees
Labels

Comments

@FrankBro
Copy link

FrankBro commented Aug 23, 2018

λ> let div = n d -> if d == 0 then DivBy0 {} else Ok (n / d)
λ> :type div
forall r. (r\Ok) => Int -> Int -> <DivBy0 : {}, Ok : Int | r>

Shouldn't the type of div be

forall r. (r\Ok\DivBy0) => Int -> Int -> <DivBy0 : {}, Ok : Int | r>

I noticed that if I swap Ok and DivBy0, I get

λ> let div = n d -> if d == 0 then Ok (n/d) else DivBy0 {}
λ> :type div
forall r. (r\DivBy0) => Int -> Int -> <Ok : Int, DivBy0 : {} | r>
@willtim
Copy link
Owner

willtim commented Aug 24, 2018

Hi, I'm on holiday at the moment, but will try to take a look soon. This is indeed a bug and it was probably introduced by the rank-2 types support introduced by commit dedfae1. I will keep you posted!

@willtim
Copy link
Owner

willtim commented Aug 24, 2018

Hi, I found the issue. If you look at line 164 in the file src/Expresso/Type.hs there is a TODO because I knew this line was wrong at the time I wrote it (I was intending to prune the substitution map elsewhere). If you change the line to read "Just t -> apply s t", the issue will be fixed. I will commit a proper fix when I get back from holiday and add some more tests. Many thanks!

@willtim
Copy link
Owner

willtim commented Aug 24, 2018

I've committed a quick fix via the github web UI: ea82773

@willtim willtim self-assigned this Aug 24, 2018
@willtim willtim added the bug label Aug 24, 2018
willtim added a commit that referenced this issue Sep 20, 2018
@willtim
Copy link
Owner

willtim commented Sep 20, 2018

Test case for this pushed

@willtim willtim closed this as completed Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants