-
-
Notifications
You must be signed in to change notification settings - Fork 103
Improve occurrence typing on Listof types #15
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
Conversation
Closes PR 14947
These don't seem to be necessary Fixup commit
Fixup commit
It turns out the special case that I added initially for this PR was unnecessary. Changing the base environment types is enough. Unfortunately, the tests don't pass yet because the change to the type of
which boils down to this inference test failure (a new test that I added):
It seems like this case should succeed though, so I'm not sure what's going on. I'll look into it, but if anyone else has any ideas let me know. |
@takikawa what's the status here? |
@samth the status is that I'm stuck on the inference problem above. I'll update this PR when I fix it. |
Note that this is a backwards incompatible change, because instantiating the functions with |
Is this worth keeping open at this point? |
This PR and the associated discussion may be useful to reference in the future, but leaving it as "open" seems incorrect at this point. |
This PR improves occurrence typing to use
car
/cdr
information forListof
types. It also simplifies the types of operations likecar
and improves type inference a little bit.Code review concerns: was there any reason this wasn't done before? (unsound maybe?) The new type for
car
seems simpler (only one case instead of two) but is it harder to understand for users?Also, I haven't actually adjusted the types for all of the
car
variants yet (the changes are straightforward). I'll do that if code review checks out.