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

Proposal for type restrictions to limit the methods that can be called on the argument or ivar #4010

Open
spalladino opened this issue Feb 8, 2017 · 7 comments

Comments

@spalladino
Copy link
Contributor

TL;DR This currently compiles, even though Enumerable has no #[] method.

def foo(x : Enumerable)
  x[0]
end

foo([1])

This proposal is for the above to fail to compile, while the following should compile:

def foo(x : Array(Int32))
  x[0]
end

def bar(x)
  x[0]
end

foo([1])
bar([1])

As of 0.20.5 and since the earliest versions, when Crystal has to compile a method call, it will compile a specific version of that method specialised with the exact types of the arguments. For example:

def foo(x)
  x + x
end

foo(1) # => will compile a foo(Int32) behind the scenes
foo(2) # => will reuse the previous one
foo("string") # => will compile a foo(String) behind the scenes

As type restrictions were introduced, they were used for selecting which method would be invoked, based on the types of the arguments; but the same behaviour of compiling a copy of the method for each specific type is maintained. Hence:

def foo(x : Int)
  x + x
end

def foo(x : String)
  x + x + x
end

foo(1) # => will compile a foo(Int32) using the first version
foo(2) # => will reuse the previous one 
foo("string") # => will compile a foo(String) using the second version

However, note that type restrictions are currently only used for choosing which method is called, and not for actually restricting the methods that can be invoked on an argument. This means that the following compiles, since Crystal will behind the scenes compile a copy of the method specialised in Array(Int32):

def foo(x : Enumerable)
  x[0]
end

foo([1]) # => it works (now)

Though it's comfortable that the compiler notices that in all uses of foo the methods required from its argument are satisfied, this can lead to further problems down the road. For instance, an issue would be someone creating a shard with the function foo, annotating and documenting it as expecting an Enumerable, testing it with an Array (it's an instance of Enumerable, isn't it?), and happily shipping the library, which will break for any client who actually uses it with a non-indexable Enumerable.

I propose that annotating a method with type restrictions should not only be limited to choosing which method to invoke, but also to restrict the methods invoke on the argument to be present in the annotated type. This means that if x is annotated as Enumerable, then only Enumerable methods should be invoked on it within foo, no matter the actual uses of foo.

Note that this does not imply to remove the current behaviour of creating a copy of the method for each different usage of the function. This approach has the advantage of having specialised functions with more direct dispatches, which yield better performance. I'm just proposing to add a check to restrict that all methods run on an argument do belong to its type. Also, if the argument is not annotated with any type, the behaviour should remain the same as of now.


A similar reasoning can also be applied to methods called on ivars typed as modules, even though the parts affected in the compiler are different. Currently, if all classes that include a module implement a method foo, even if the module does not define foo, then it can be invoked on an ivar of that module. For example:

module Moo
  def moo; end
end

class Foo1
  include Moo
  def foo; end
end

class Foo2
  include Moo
  def foo; end
end

class Bar
  def initialize(@moo : Moo); end
  def bar
    @moo.foo # => compiles because all "includers" of Moo define a method #foo
  end
end

Again, the code above works, but is easily breakable should any class includes Moo anywhere else in the code, without defining foo. Additionally, this introduces some extra time-consuming complexity con the compiler to keep track that all includers of Moo (though not necessarily Moo itself) define foo.

The proposal is, for the above to work, to force Moo to have a method foo defined, even as abstract; this would allow for modules to be actually used as interfaces in the language.

Comments?

@ysbaddaden
Copy link
Contributor

That makes sense, yet, I'm afraid it may push for lots of interfaces (see Rust traits). What if I need an Enumerable and [] but I don't want to restrict to Array specifically? Do I need a second type? AFAIK we can't restrict on having two types at the same, so I'll need an artificial type that mixes both types, and now anything that uses my method must include this artificial type :(

module Accessible(T)
  abstract def [](index) : T
end

module AccessibleEnumerable(T)
  include Accessible(T)
  include Enumerable(T)
end

class Bar(T)
  include AccessibleEnumerable(T)

  def [](index)
    # ...
  end

  def next
    # ...
  end
end

def foo(bar : AccessibleEnumerable)
  bar.map_with_index do |value, index| 
    { value, bar[index + 1]? }
  end
end

foo(Bar(Int32).new)

@spalladino
Copy link
Contributor Author

@ysbaddaden I can't think of many cases where you'd need for an interface plus a few more methods. And still, if you do, you could simply not annotate the argument and it would work right away:

class Bar(T)
  def [](index)
    # ...
  end

  def next
    # ...
  end
end

def foo(bar)
  bar.map_with_index do |value, index| 
    { value, bar[index + 1]? }
  end
end

foo(Bar(Int32).new)

@mverzilli
Copy link

AFAIK we can't restrict on having two types at the same

Which makes a nice case for eventually adding intersection types :) (see for example: https://flowtype.org/docs/union-intersection-types.html#intersection-example)

@ysbaddaden
Copy link
Contributor

I'm not against the change; it doesn't make sense to restrict on type A, yet access methods that aren't defined on A.

I want to outline potential issues, so we can make a conscious choice of tradeoffs. Maybe the apparition of many interface types; or maybe we will control them in the core/stdlib, so it won't go outboard —I'm seriously scared of Rust traits.

@mverzilli let's open an RFC for a A & B type restriction!

@RX14
Copy link
Contributor

RX14 commented Feb 8, 2017

Only on type restrictions though right? I can't think how intersection types would have a more general apparition like unions do?

@HertzDevil
Copy link
Contributor

Intersection types are covered in #2404 and restrictions in #7534.

@RX14
Copy link
Contributor

RX14 commented Mar 21, 2021

I think intersection types depend on this.

Either way, this would be a 2.0 feature (please!) at this point.

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

No branches or pull requests

5 participants