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

if nil guard doesn't work for instance variable #477

Closed
stefanwille opened this issue Mar 12, 2015 · 10 comments
Closed

if nil guard doesn't work for instance variable #477

stefanwille opened this issue Mar 12, 2015 · 10 comments

Comments

@stefanwille
Copy link

According to the docs (http://crystal-lang.org/docs/syntax_and_semantics/if_var.html) the compiler will assume that a variable 'var' can not be nil after a 'if var':

  if var
    # var is guaranteed not to nil:
    var.some_method ...
  end

This works for local variables, but fails for instance variables:

class Stuff
  def foo
    @instance_variable = [] of String
  end

  def bar
    if @instance_variable
      # Should be ok, but compiler complains:
      @instance_variable << "bar"
    end
  end
end

stuff = Stuff.new
stuff.foo
stuff.bar

For this example, I get a compile time error:

Error in ./instance-variable-nil.cr:17: instantiating 'Stuff#bar()'

stuff.bar
      ^~~

in ./instance-variable-nil.cr:10: undefined method '<<' for Nil

      @instance_variable << "bar"

I would expect this to work the same as for local variables.

@stefanwille
Copy link
Author

A temporary workaround is to use a local variable.

@JacobUb
Copy link
Contributor

JacobUb commented Mar 12, 2015

asterite explained this here #398 (the first reply)

@jhass
Copy link
Member

jhass commented Mar 12, 2015

I thought it was mentioned somewhere but here's why it doesn't:

An instance variable is shared data, in a multi-threaded context it's possible that the condition is checked and matches, then control is given to another thread who sets the variable to nil and then control is given back to the thread that checked the variable and that is assuming now that it's not nil while it is. Also given that we do have parallelism these days, that scenario becomes even more likely.

A             |    B
if @foo 
                   @foo = nil
  @foo.bar

@stefanwille
Copy link
Author

Thanks again for the quick reply.

I take it that this is the explanation (copied from asterite):

Basically, in Crystal the type flow only applies to local variables. So you must first assign the instance variable to a local variable. Then if you do is_a?(...) the compiler will know that inside the if the variable will be of that type. Anything more complex than that and the compiler won't understand it.

It would be cool to mention in the docs that the "if var" guard only works for local variables. Or to allow instance variables too, as this seems fairly easy for "if var".

@asterite
Copy link
Member

@jhass Bingo!

@stefanwille It's explained in the docs: http://crystal-lang.org/docs/syntax_and_semantics/if_var.html (scroll to the bottom). It says "The above logic doesn’t work with instance variables, class variables or global variables" and explains why.

@stefanwille
Copy link
Author

@asterite Alright, found it, thanks!

@ysbaddaden
Copy link
Contributor

That being said, assigning to a local variable is a bit boring, and unexpected by newcomers, especially when there are no other methods setting @var to nil (or another type).

I'm not sure, but, does Crystal complains when accessing @var through a getter? Because the same scenario by @jhass would apply.

@jhass
Copy link
Member

jhass commented Mar 12, 2015

Pretty sure it does.

@ysbaddaden
Copy link
Contributor

Yes it does, because getter is merely a def foo; @foo; end. There must have been another case that made me think otherwise. Sorry for the noise.

@JacobUb
Copy link
Contributor

JacobUb commented Mar 12, 2015

@ysbaddaden maybe you were thinking about this https://github.com/manastech/crystal/blob/master/src/object.cr#L213 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants