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

Abstract method implemented via extend self not considered to be implemented in some circumstances #8096

Closed
JacobUb opened this issue Aug 17, 2019 · 7 comments · Fixed by #8098
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic

Comments

@JacobUb
Copy link
Contributor

JacobUb commented Aug 17, 2019

This worked in 0.29.0:

module ICallable(T)
  abstract def call(foo : Proc(T))
end

module Caller
  extend self
  extend ICallable(Nil)

  def call(foo : Proc(Nil))
  end
end

In 0.30.1 it says that the abstract method call must be implemented by Caller:Module.

  • Compiles by changing def call to def self.call.
  • Also compiles if generics are not involved.
@asterite
Copy link
Member

So, when you use extend it's like adding the module's methods as class methods. Essentially we can replace extend ICallable(Nil) in module Caller with this:

module Caller
  extend self
  
  # extend ICallable(Nil)
  abstract def self.call(foo : Proc(T))
  def call(foo : Proc(Nil))
  end
end

and as you can see the abstract def self.call is not implemented by Caller and then you get an error. That's why the error is also gone if you then do def self.call(...).

This issue is invalid.

Did you maybe mean to use include?

@JacobUb
Copy link
Contributor Author

JacobUb commented Aug 18, 2019

Therefore this code:

module ICallable
  abstract def call(foo)
end

module Caller
  extend self
  extend ICallable

  def call(foo)
  end
end

shouldn't work either? Because it's the same thing minus the types, and it works.

@asterite
Copy link
Member

abstract def for class methods doesn't make sense. Maybe it's a bug. I think abstract class methods shouldn't compile, but here you get one through extend.

What are you trying to do?

@asterite asterite reopened this Aug 18, 2019
@asterite
Copy link
Member

I would also try to avoid extend, in the future I can see us removing it.

@JacobUb
Copy link
Contributor Author

JacobUb commented Aug 18, 2019

It's not really an abstract class method. It's just an abstract method that just happens to be implemented by a module that extends itself. In this case, Caller itself is a ICallable for the purpose of for example adding it to an Array(ICallable) so it makes sense that it should conform to the protocol established by ICallable.
It's not something that I'm trying to now, it's code that had been working since late 2015 and it stopped working now.

@asterite
Copy link
Member

O, actually, I think you are right. extend self should bring the type's methods into the class scope. Then the def self.call should be implemented but for some reason it doesn't.

Workaround for now is to remove the abstract method... I guess.

@asterite asterite added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic labels Aug 18, 2019
@asterite
Copy link
Member

@exilor Fixed! And sorry for the initial misunderstanding.

@RX14 RX14 closed this as completed in #8098 Sep 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants