lizmat / Method-Also

Method::Also - add "is also" trait to Methods
Artistic License 2.0
2 stars 2 forks source link

When an alias is specified in a role, it is not recognized #1

Closed Xliff closed 5 years ago

Xliff commented 5 years ago

See the following code:

use Method::Also;

my role C {

    proto method my_method (|)
        is also<my-method>
    { * }

    multi method my_method (Str $a) {
        say "Str";
    }

    multi method my_method (Int $b) {
        say "Int";
    }
}

my class B { also does C; }

B.new.my_method(42);
C.new.my_method('b');
B.new.my-method(42);   # Using alias
C.new.my-method('b');  # Using alias

Output:

Int
Str
Cannot resolve caller my_method(B:D, Int:D); Routine does not have any candidates. Is only the proto defined?

Compare that behavior to the one when using a pure Class:

use Method::Also;

my class D {

    proto method my_method (|)
        is also<my-method>
    { * }

    multi method my_method (Str $a) {
        say "Str";
    }

    multi method my_method (Int $b) {
        say "Int";
    }
}

D.new.my_method(42);
D.new.my_method('b');
D.new.my-method(42);   # Using alias
D.new.my-method('b');  # Using alias

Output:

Int
Str
Int
Str

perl6 --version:

This is Rakudo version 2019.07.1-130-ga7302f0a4 built on MoarVM version 2019.07.1-72-g352ae27e4
implementing Perl 6.d.

Just for kicks, I tried to bisect this on IRC. The results are here:

https://gist.github.com/Whateverable/6ad41ee17e9251bd8ea61f3f7674e004

It looks like this has been an issue since the 2015 release in one way or another.

lizmat commented 5 years ago

Will try to look at this over the weekend.

lizmat commented 5 years ago

Looks like this problem only exists when doing an is also trait on a proto in a role. Looking into this further.

lizmat commented 5 years ago

I'm not sure whether this is a problem in Rakudo's MOP, or that Method::Also is abusing the MOP in such a way that this doesn't work. Anyways, what I found so far:

  1. adding a method to a role, does not immediately add the method, but adds it to a list to be added on composition.
  2. if the method is not a proto, then everything is ok
  3. if the method is a multi, the method gets added as an "only" method
  4. if we add the proto, it gets added as an "only" method

This last thing is causing the issue. I'm not sure how this can be made to work, because by the looks of this, when we add a proto in the role definition, it works. But if we add the proto using the MOP, it doesn't. Not sure whether it's incorrect invocation of the MOP, or that it is really a problem in the MOP itself. I'm not fully grokking the MOP in that respect, so this may require more research.

Xliff commented 5 years ago

Could Method::Also wait for class composition in the case of a Metamodel::ParametricRoleHOW?

Xliff commented 5 years ago

@lizmat: Here's a working solution!

lizmat commented 5 years ago

Thanks for that. But it feels to me we're working around a deficiency in role composition? And that the proper place to fix it would be there?

Xliff commented 5 years ago

Sure!

Should I put this up as a bug for rakudo?

lizmat commented 5 years ago

Please do. And perhaps see what needs to be changed in the role composition to Method::Also work as is? And make a PR for that? That would be brill!

Xliff commented 5 years ago

Sure thing!

Xliff commented 5 years ago

Actually, with a few minor changes, I think that snippet would work just fine. I will set it up as a PR for your review.