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: fix type flow of var in if with && #7785

Merged
merged 1 commit into from
May 15, 2019

Conversation

asterite
Copy link
Member

Fixes #7434

This one is harder to explain so I'm going to skip that.

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels May 15, 2019
@asterite asterite merged commit ca93f57 into crystal-lang:master May 15, 2019
@asterite asterite added this to the 0.29.0 milestone May 15, 2019
@asterite asterite deleted the bug/compiler-if-and-var branch May 15, 2019 23:09
end

foo
), inject_primitives: false) { int32 }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand your test, in all cases the return type can only be int32, I think you want to return false or some other type?

Copy link
Member

Choose a reason for hiding this comment

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

That's the point: This test ensures that the compiler recognizes that the return type is always Int32, because the if branch can never be reached.

Copy link
Contributor

Choose a reason for hiding this comment

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

oooh ok I think I get it, it'd be far easier to understand if we could do:

if true && (y = 1 || nil)
end
assert_type(y, Int32)

Or maybe just do:

assert_type(%(
  if true && (y = 1 || nil)
  end
  y
  ), inject_primitives: false) { int32 }

?

Copy link
Member Author

Choose a reason for hiding this comment

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

But in your examples y ends up nilable. It's the return in the else branch that makes it non nilable. But that what was not working well (when in a right side of &&).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting more than one local var in "return unless ..." leaves it nilable
4 participants