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

Warning with delegate and abstract method #8250

Open
kostya opened this issue Sep 29, 2019 · 4 comments
Open

Warning with delegate and abstract method #8250

kostya opened this issue Sep 29, 2019 · 4 comments

Comments

@kostya
Copy link
Contributor

kostya commented Sep 29, 2019

is this ok?

abstract class A
  abstract def bla : Int32
end

class Bla
  def bla : Int32
    1
  end
end

class B < A
  def initialize
    @bla = Bla.new
  end

  delegate :bla, to: @bla
end

p B.new.bla
In 1.cr:16:3

 16 | delegate :bla, to: @bla
      ^
Warning: expanding macro


There was a problem expanding macro 'delegate'

Called macro defined in /usr/local/Cellar/crystal/0.31.0_1/src/object.cr:1208:3

 1208 | macro delegate(*methods, to object)

Which expanded to:

    1 |
    2 |
 >  3 |         def bla(*args, **options)
    4 |           @bla.bla(*args, **options)
    5 |         end
    6 |
    7 |
    8 |           def bla(*args, **options)
    9 |             @bla.bla(*args, **options) do |*yield_args|
   10 |               yield *yield_args
   11 |             end
   12 |           end
   13 |
   14 |
   15 |
   16 |
Warning: this method overrides A#bla() which has an explicit return type of Int32.

Please add an explicit return type (Int32 or a subtype of it) to this method as well.

The above warning will become an error in a future Crystal version.

A total of 1 warnings were found.
@asterite
Copy link
Member

Another case that hints that abstract methods in this language should maybe disappear

@bcardiff
Copy link
Member

The same will happen for any macro that expands a method. They might need a way to inject the return type in order to comply with the current abstract method check.

A way to unlock the delegate macro is to allow passing the returning type.

delegate :bla, to: @bla, returning: Int32
delegate bla : Int32, to: @bla

# NOTE: proposal / not working example

But it takes some of the fun.

The property macro does allow return types already.

As I mentioned at #8232 (comment) if the check is reframed as an after check to enforce that the contract of the abstract method is hold then this issue could be resolved without changes to the delegate macro.

@asterite
Copy link
Member

Maybe an after check could work. The problem is that when I tried to do that the compiler would already complain of the type mismatch in the regular type inference phase, or complain about the missing method. In summary, the compiler will complain, it's just that the error message will be slightly different.

@bcardiff
Copy link
Member

I understand that the compiler will probably complain before a delayed abstract check. If the offending override is used in a way it bothers. The explicit return type helped to catch this earlier.

Maybe the relaxation is that if there is an explicit return type it needs to match the abstract def. That way the generated method that will probably work by construction will not need a workaround to annotate a return type.

A compiler tool could be in charge of suggesting the explicit return types.

(throwing ideas to find the sweet spot of freedom and contracts here)

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

4 participants