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

Inherit class vars (only their type, not their value) #2761

Merged
merged 1 commit into from
Jun 7, 2016

Conversation

asterite
Copy link
Member

@asterite asterite commented Jun 6, 2016

This fixes #2458

This PR makes class variables be inherited from superclasses and included modules. This only means that a class variable is available in a subclass, with the same type as a superclass class variable, but their value is not shared. If you are familiar with Ruby class instance variables, this is the same. This works for modules too (class variables are inherited from including modules). If there are type conflicts the compiler gives an error (there are a few specs in this PR, you can check them).

For example:

class Foo
  @@var = [1, 2, 3]

  def self.var=(@@var)
  end

  def self.var
    @@var
  end
end

class Bar < Foo
end

p Foo.var # => [1, 2, 3]
p Bar.var # => [1, 2, 3]

Foo.var << 4

p Foo.var # => [1, 2, 3, 4]
p Bar.var # => [1, 2, 3]

Bar.var << 5

p Foo.var # => [1, 2, 3, 4]
p Bar.var # => [1, 2, 3, 5]

[Foo.new, Bar.new].each do |foo|
  # Prints [1, 2, 3, 4] first, then [1, 2, 3, 5]
  p foo.class.var
end

Note that the initializer of @@var is not shared between Foo and Bar, they are different instances.

If you think about it, it's very similar to instance variables. When you have @foo = [1, 2, 3] in a class, every instance you create gets a new instance of that array. If you modify @foo in one instance, it doesn't affect other instances. This is the same now for class variables.

Also note that you can iterate a class hierarchy and access a class variable, and the dispatch happens. So this also fixes not being able to access a class variable through a Foo+ type.

There are no plans to include a feature like Ruby's class variables, for two reasons:

  1. I believe they are considered dangerous and bug-prone in Ruby (please correct me if this is not the case)
  2. One can emulate them by using a separate class/module to hold the class variable. Consider the example in Compiler can't infer seemingly obvious type #2757. It seems that @@strings was supposed to be shared between all descendants. To solve this, instead of providing access to @@strings, one can define a method strings that accesses @@strings in another nested module. This also has the advantage of not having to expose @@strings.

Thoughts before merging this? :-)

self_type_id = type_id(llvm_self, owner)
read_function_name = "~#{class_var_global_name(owner, class_var.name)}:read"
func = @main_mod.functions[read_function_name]? ||
create_read_virutal_class_var_ptr_function(read_function_name, node, class_var, owner)
Copy link
Contributor

Choose a reason for hiding this comment

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

virutal -> virtual (multiple times)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thaskn! It's taht "atni-feautre" of the bairn taht ltes you raed stuff lkie taht :-(

@jhass
Copy link
Member

jhass commented Jun 6, 2016

If you are familiar with Ruby class instance variables, this is the same.

To clarify, the same in regard of inheriting the type, not the same in regard of shared data.

There are no plans to include a feature like Ruby's class variables

👍

"class variable '@@x' of Bar is already defined as Int32 in Foo"
end

it "errors if redefining class var type in subclass, with guess" do
Copy link
Member

Choose a reason for hiding this comment

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

one could reopen Foo and declare @@x : Int32 | Char. So there is nothing wrong with the following code. Guessing is not the same as declaring. Maybe not right now, but I think crystal could be able to compile this code on it's own.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only reaction for me - I'd like to be able to redefine classvar with another type in sub class. One might have a common use case, so inherited for convenience, and then some exceptional cases. Feels like something that could come up, but I have no concrete use case, so might be fumbling in the dark here...

Copy link
Member Author

Choose a reason for hiding this comment

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

@bcardiff @ozra One can reopen a class and redeclare a class variable. What you can't do is declare a different type in a subclass. This is the same as with instance variables.

Copy link
Member

Choose a reason for hiding this comment

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

@asterite notice that my comment is about the snippet that does not have type declaration. Only the assignments. github shows the spec before line 393. Maybe we should track in different the type declaration and the assignments without type annotations.

@asterite asterite force-pushed the feature/class_vars branch from f8433d1 to 3629e13 Compare June 6, 2016 16:36
@asterite asterite merged commit 4e8aa74 into master Jun 7, 2016
@asterite asterite deleted the feature/class_vars branch June 9, 2016 01:12
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

Successfully merging this pull request may close these issues.

Inherit class variable types from ancestor
5 participants