lizmat / Method-Also

Method::Also - add "is also" trait to Methods
https://raku.land/zef:lizmat/Method::Also
Artistic License 2.0
2 stars 2 forks source link

Make sure method `handles` trait is not re-applied #4

Closed vrurg closed 2 years ago

vrurg commented 2 years ago

rakudo/rakudo#4994 resulted in an unpleased issue where duplicating the same method object under a different name caused lazy handles application to be repeated and delegation methods re-added again. For now I have added a check which prevents this to happen if the method duplication is detected, but it's an additional loop iterating over already installed ones.

To fix this issue I have added :$handles parameter to add_method. Currently it would serve as a mere optimization if negated. But in the future (likely after this commit makes it into Method::Also release) I plan to remove the loop and make any third party module to make sure they don't cause delegation duplication.

lizmat commented 2 years ago

Testing fails with "Unexpected named argument 'handles' passed". Wouldn't that imply that a. it would need an updated Rakudo, and b. this would make this module fail on older rakudos?

vrurg commented 2 years ago

I always forget that for NQP nameds are not optional... Yes, it is to support rakudo/rakudo#4997. And yes, the existing Rakudo versions are a problem. I'm trying to figure out a better way, but the named is the best. All other approaches require tracking of methods with handles in one way or another. Either it's a loop (which I use as a workaround for the issue currently), or an additional attribute on Metamodel::MethodContainer which I don't like either.

vrurg commented 2 years ago

Made it backward compatible.

lizmat commented 2 years ago

Thanks!