Crystal: Instance variable type inference fails after type reduction in branch

Created on 10 Apr 2017  路  8Comments  路  Source: crystal-lang/crystal

class Test
  getter number = 1

  def incr(set = nil)
    if set
      @number = set
    else
      @number += 1
    end
  end
end

t = Test.new
t.incr

Produces:

Error in line 14: instantiating 'Test#incr()'

in line 4: instantiating 'incr(Nil)'

in line 8: undefined method '+' for Nil (compile-time type is (Int32 | Nil))

(line 8: @number += 1)

This can be worked around by supplying a type restriction first in the getter macro (thanks @RX14 )

class Test
  getter number : Int32 = 1
  # ...
end

also: Checking types of set and @number

wontfix

All 8 comments

The compiler infers types of instance variables from simple assignments such as @number = exp, without doing a complex analysis such as type filtering. Here the compiler doesn't see that set is behind if set, at least not in this dummy inference pass.

Another workaround:

class Test
  getter number = 1

  def incr
    @number += 1
  end

  def incr(set)
    @number += set
  end
end

t = Test.new
t.incr

I'm closing this because I really doubt will "fix" this.

I don't know enough about crystal's wetworks to know what is possible, and I don't think anything needs to be "fixed" to make this code run as-is, but I do think there's some room for improvement. What is most confusing about this is the error message and debugging this (this occurred to me in a much more complex application, boiled down to this example)

Is there maybe something that could be done to this effect to flag the user?

TIL _wetwork_, thx @z64! :)

Some possibilities:

  1. Don't infer types from assignments inside if, while, etc.
  2. Force instance variable type declaration, so that the compiler and the user don't have to "guess" anything

I'm every time more convinced about the second option. Saving those typed chars for type declarations is probably not worth the trouble.

Sorry @asterite, I don't get your point... Isn't number 2 what we already do? Or instance variable != ivar?

I think guessing instance variables should be based on the constructor and only the constructor, and only for assignments outside if/while or any block. We should support def initialize(@foo : String) at minimum.

@mverzilli No, because this compiles and there's no type annotation:

class Foo
  property x = 1 # x is inferred to be Int32
end

Then I agree, it doesn't seem terrible

Was this page helpful?
0 / 5 - 0 ratings

Related issues

rdp picture rdp  路  112Comments

asterite picture asterite  路  78Comments

malte-v picture malte-v  路  77Comments

MakeNowJust picture MakeNowJust  路  64Comments

sergey-kucher picture sergey-kucher  路  66Comments