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: use freeze_type as node type if node doesn't have a type #7161

Merged
merged 1 commit into from Dec 16, 2018

Conversation

Projects
None yet
3 participants
@asterite
Copy link
Contributor

asterite commented Dec 8, 2018

Fixes #7160

When the compiler sees a method return type it will try to resolve it and then set it as the def's @freeze_type. This instance variable holds a type that eventually must match the type that is normally inferred. The compiler doesn't use @freeze_type for anything else.

This PR changes that: if there's no inferred type for a node but it has a @freeze_type, the node's type will be "inferred" to be that type. So if you say a method's return type is Int32, even before the compiler can deduce its real type and see if it matches Int32 it will have an Int32 type. This allows using the method in a recursive method and still be able to fully "infer" it's type (it's a bit of cheating because we are saying the type, but previously this didn't work).

One example is the issue this PR solves:

class Tree
  def initialize(@value : Int32, @children : Array(Tree))
  end

  def sum : Int32
    @value + @children.each.map(&.sum).sum
  end
end

Tree.new(1, [] of Tree).sum

Here the compiler will try to type Tree.new(...).sum and for that it will need to type @children.each.map(&.sum), which needs the type of sum, which isn't computed yet. The compiler before this PR would fail. With this PR it will succeed because we now know the type of sum.

Don't worry: the compiler will later check that the type is effectively what we told it.

An interesting side effect of this change is that in these recursive scenarios the return type isn't a hint anymore, it's a type that's used for inference. For example, this works as well:

class Tree
  def initialize(@value : Int32, @children : Array(Tree))
  end

  def sum : Float64
    @value + @children.each.map(&.sum).sum
  end
end

Tree.new(1, [] of Tree).sum

Because @value is Int32 and sum is told to be Float64, the right-hand side expression @children... is deduced to be Float64, and Int32 + Float64 gives Float64, so this compiles fine. It's a bit spooky, but it's fine :-)

@bcardiff Would it be possible to run the script that checks the compiler works fine for many of the major crystal projects? This is a simple change, all specs pass, but I wonder if it could break something that I'm not seeing.

@sdogruyol
Copy link
Member

sdogruyol left a comment

Thank you @asterite 👍

P.S: I ran this against Kemal master 💯

@RX14 RX14 added this to the 0.27.1 milestone Dec 16, 2018

@RX14 RX14 merged commit dfe7cf8 into crystal-lang:master Dec 16, 2018

4 checks passed

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment