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

Compiler: merge procs with the same arguments type and Nil | T return type to Nil return type #7527

Merged
merged 1 commit into from Mar 8, 2019

Conversation

Projects
None yet
5 participants
@asterite
Copy link
Member

commented Mar 8, 2019

Fixes #3655

The idea here is that a proc that returns any type T is compatible with a proc with the same argument types that returns Nil. The union type of those things is just the type of the proc that returns Nil, as in "in the general case we don't know anything about the value being returned".

This is a rule that's currently being applied in a few places, this fixes the language to apply it in more (all?) places.

@asterite asterite merged commit 0900562 into crystal-lang:master Mar 8, 2019

5 checks passed

ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@asterite asterite added this to the 0.28.0 milestone Mar 8, 2019

@paulcsmith

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2019

Incredible! That was so fast. Thank @asterite

@asterite

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2019

@paulcsmith Glad to help! 😊

It would be great if you can try it out in the code you have, if you are familiar with compiling the compiler (or maybe now there are nightlies that can make this easier).

@asterite asterite deleted the asterite:feature/proc-nil branch Mar 30, 2019

@Blacksmoke16

This comment has been minimized.

Copy link
Contributor

commented Apr 19, 2019

@asterite This PR ended up breaking Athena. (The commit before this was merged works fine). I'll see if i can come up with a reduced example of what is breaking.

EDIT: It was prob broke before and this "fixed" it, I figured out the cause and the solution was something I was going to do anyway, so nvm.

EDIT2: No, something is deff wrong, but im having trouble coming up some code.

EDIT3: Made an issue, #7698

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.