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

Make delegate to be a macro implemented by the compiler #9927

Open
Sija opened this issue Nov 17, 2020 · 12 comments
Open

Make delegate to be a macro implemented by the compiler #9927

Sija opened this issue Nov 17, 2020 · 12 comments

Comments

@Sija
Copy link
Contributor

Sija commented Nov 17, 2020

I'd actually prefer if we change delegate to be a macro implemented by the compiler. Then whenever we find a call to that delegated method, we replace it with the delegation. It will solve this problem and others (like capturing block arguments not possible right now), even though it's a "magic" solution.

Originally posted by @asterite in #9683 (comment)

Refs #9683, #9682, #8250, #6731, #6473

@BrucePerens
Copy link

BrucePerens commented Nov 28, 2020

I was able to get pretty far with making a better delegate, it is type-safe and copies the argument list from the delegated-to method, and it handles blocks, and &block with type restriction. But the syntax is slightly different. This version requires that you provide a class name, so A.delegate2 method-name, to: instance-of-A.

class Object
  macro delegate2(name, to)
   {% if m = @type.methods.find &.name.id.==(name.id) %}
     {% if m.block_arg %}
       {% args = (m.args + ["&#{m.block_arg}".id]).splat %}
       {% yield_args = m.block_arg.restriction.inputs.map_with_index{|a,index| "arg#{index}".id}.splat %}
     {% else %}
       {% args = m.args.splat %}
       {% yield_args = "*args".id %}
     {% end %}
     def {{name}}({{args}}) {% if !m.return_type.is_a?(Nop) %}: {{m.return_type}} {% end %}
       {{to}}.{{name}}({{m.args.map(&.internal_name).splat}}) {% if m.accepts_block? %}{ |{{yield_args}}| yield({{yield_args}})}{% end %}
     end
   {% else %}
     {% raise "delegate: Method \"#{name}\" wasn't found in \"#{@type.name}\"." %}
   {% end %}
   {% debug %}
  end
end

class A
  def foo(a : Int32, b : Int32, &block : Int32, Int32 -> String) : String
    yield a, b
  end
end

class B
  @a = A.new
  A.delegate2 foo, to: @a
end

b = B.new
p b.foo(1, 2) { |i, j| "Hello" }

Thanks to @Blacksmoke16 for rewriting my first, too-wordy, version.

@BrucePerens
Copy link

If someone could show me how to get rid of the A in A.delegate2 foo, to: @a, the above would solve all the issues that I think @Sija has.

@asterite
Copy link
Member

There's no way to do it because when the macro runs type information for the instance variable, or whatever expression you pass, is not available.

Also, the current delegate macro works even before the method is declared on the target type. Plus it works if the target is a union. The delegate macro can really be best implemented by having the compiler implement it.

@BrucePerens
Copy link

BrucePerens commented Nov 28, 2020

@asterite Wouldn't this work if the macro language included a typenode() operator, which would return a TypeNode referring to the argument? So that one could use typenode(@a).methods? I guess it would have to synthesize a TypeNode for unions.

I agree it would not work on methods that were not yet defined. But you need the type information, in the compiler, to make type-restricted yield work properly, and having the type information gets rid of the need to kludge around functions that must only have one argument, like ==.

@asterite
Copy link
Member

asterite commented Nov 28, 2020

When the macro is executed the type information of instance variables isn't yet computed. So typenode(@a) won't work because there's no type yet at that moment.

@BrucePerens
Copy link

OK, so the only way for this to work for all cases is for it to be a compiler keyword, and for generation of the delegator method to be deferred until the delegated-to method is defined.

@asterite
Copy link
Member

I think so. It could also be implemented by method missing. The problem is that there's only one method_missing. Maybe it would be nice to be able to register handlers for method_missing at the language level...

@asterite
Copy link
Member

Actually, we could make each method_missing be able to call the previous definition if it exists, so you can form a chain. I might play with this idea...

@BrucePerens
Copy link

Please code any new delegate keyword so that it is possible to separately name the delegator method. I have a use case of extending delegate to wrap and unwrap arguments and return values, in https://github.com/BrucePerens/recursive_generic . At least unless/until recursively-defined aliases get fixed.

@asterite
Copy link
Member

Well, it seems there's no way to capture a block with method_missing.

I personally think we should remove delegate. It's very easy (though admittedly tedious the first time) to define the actual method that delegates the call. And once you write it there's no need to touch that code again. Well, I guess if the underlying method changes... but that's an easy change too.

@BrucePerens
Copy link

BrucePerens commented Nov 28, 2020

It's not so bad to insist on the user providing the type information, then, as I currently do in delegate2. Even if the user has to find the union member providing a method, and if the method has to already be defined.

@asterite I've noticed you're eager to remove many language features. And, I guess, can't convince the team.

I ran into another issue, which is that my delegate2 must delegate all methods by that name in the target.

I continue to work on this, and will make it a shard when I'm done. https://github.com/BrucePerens/delegate

@asterite
Copy link
Member

Yes, I'd like to remove things that don't work 100% of the time. They can be added later when we find a way to make them work in all cases.

That said, I use delegate a lot in the compiler and it always worked fine.

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