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

.wrap of a method breaks callsame/nextsame/etc. #2178

Closed
vrurg opened this issue Aug 4, 2018 · 2 comments
Closed

.wrap of a method breaks callsame/nextsame/etc. #2178

vrurg opened this issue Aug 4, 2018 · 2 comments

Comments

@vrurg
Copy link
Member

vrurg commented Aug 4, 2018

The Problem

Wrapping a method in a child class breaks MRO chaining for callsame/nextsame. Consider the following code:

     class Foo {
         has Int $.attr is rw;

         method m {
             say "<this is Foo:m>";
             callsame;
         }
     }

     class Bar is Foo {
         method m {
             say "<this is Bar::m>";
             callsame;
         }
     }

     class Baz is Bar {
         method m {
             say "<this is Baz::m>";
             callsame;
         }
     }

     Baz.^lookup( 'm' ).wrap( method { say "<this is WRAPPER:m>"; nextsame } );

     my $inst = Baz.new;

     $inst.m;

It produces the following output:

<this is WRAPPER:m>
<this is Baz::m>

Method m of class Baz cannot call the next method in MRO chain.

There is even worse case scenario if wrap is applied to Bar::m method. Replacing the wrapping line in the source with the following:

Bar.^lookup( 'm' ).wrap( method { say "<this is WRAPPER:m>"; nextsame } );

gives us:

<this is Baz::m>
Cannot invoke object with invocation handler in this context

Environment

  • Operating system: macOS 10.13
  • Compiler version (perl6 -v): This is Rakudo version 2018.06 built on MoarVM version 2018.06
@vrurg
Copy link
Member Author

vrurg commented Aug 12, 2018

There is interesting development for the situation when the method in Bar is wrapped. Adding a call to nextcallee into Baz::m:

     class Baz is Bar {
         method m {
             say "<this is Baz::m>";
             my &nc = nextcallee;
             #self.&nc;
             callsame;
         }
     }

avoid the exception about invocation handler. Instead the following output is produced:

<this is Baz::m>
<this is Foo:m>

It looks like this time callsame just got around Bar::m by simply ignoring it! By uncommenting the self.&nc call I get the expected output:

<this is Baz::m>
<this is WRAPPER:m>
<this is Bar::m>
<this is Foo:m>

but the point that it is produced by two separate calls. I.e., callsame from Bar::m did not pass the control to Foo, but from Baz::m did!

Now, the final trick: leave the self.&nc but comment out callsame in Baz::m. It raises another exception:

<this is Baz::m>
Cannot invoke this object (REPR: Uninstantiable; Callable)
  in method m at ./wrap.p6 line 23
  in block <unit> at ./wrap.p6 line 32

These tests were made with a recent rakudo checkout: This is Rakudo version 2018.06-407-g3301ddb1c built on MoarVM version 2018.06-399-g4fc267b60

vrurg added a commit to vrurg/raku-AttrX-Mooish that referenced this issue Aug 13, 2018
vrurg added a commit to vrurg/rakudo that referenced this issue Mar 7, 2019
The fix is done by implementing cooperation between dispatchers. If a
dispatcher finds out that next call in the queue is a invocation handler
with attribute $!dispatcher containing a object of descendant type of
Metamodel::BaseDispatcher it passes control over to that dispatcher by
calling either enter_with_args or enter_with_capture method. The
subsequent dispatcher passes control back to the calling dispatcher upon
exhausting its own queue.

The known problems of this fix:

- there is no clear way to determine if next call is a invocation
handler. The approach used by this fix is to wrap the attempt to read
from $!dispatcher attribute into a try block is hacky and would have
performance penalty. Must be changed as soon as more straightforward
approach is developed or found.

- The enter_* methods are currently specific to the WrapDisptacher. So
far there is seemingly no way to create a invocation handler with method
or multi dispatcher in $!dispatcher and get such handler inserted into
the WrapDispatcher queue. But neither there is no guaranty that no other
such dispatcher would be developed in the future. For this reason the
enter_* methods must be standartized and possibly moved into
BaseDispatcher class.
vrurg added a commit to vrurg/rakudo that referenced this issue Mar 9, 2019
For wrapper-like dispatchers (with WrapDispatcher being currently the
only one) we won't set explicit dispatcher for the last candidate on the
queue effectively allowing a control call like callsame/nextsame to
obtain routine's default dispatcher. This fixes a situation when a
wrapped method cannot pass control over to the next method on
MethodDispatcher queue.

rakudo#2178
vrurg added a commit to vrurg/rakudo that referenced this issue Mar 9, 2019
vrurg added a commit that referenced this issue Aug 16, 2019
Fix for #2178: .wrap breaks method dispatching for inheriting classes.
@vrurg
Copy link
Member Author

vrurg commented Aug 16, 2019

Closed via #2749

@vrurg vrurg closed this as completed Aug 16, 2019
vrurg added a commit to vrurg/rakudo that referenced this issue Sep 23, 2019
The fix is done by implementing cooperation between dispatchers. If a
dispatcher finds out that next call in the queue is a invocation handler
with attribute $!dispatcher containing a object of descendant type of
Metamodel::BaseDispatcher it passes control over to that dispatcher by
calling either enter_with_args or enter_with_capture method. The
subsequent dispatcher passes control back to the calling dispatcher upon
exhausting its own queue.

The known problems of this fix:

- there is no clear way to determine if next call is a invocation
handler. The approach used by this fix is to wrap the attempt to read
from $!dispatcher attribute into a try block is hacky and would have
performance penalty. Must be changed as soon as more straightforward
approach is developed or found.

- The enter_* methods are currently specific to the WrapDisptacher. So
far there is seemingly no way to create a invocation handler with method
or multi dispatcher in $!dispatcher and get such handler inserted into
the WrapDispatcher queue. But neither there is no guaranty that no other
such dispatcher would be developed in the future. For this reason the
enter_* methods must be standartized and possibly moved into
BaseDispatcher class.
vrurg added a commit to vrurg/rakudo that referenced this issue Sep 23, 2019
For wrapper-like dispatchers (with WrapDispatcher being currently the
only one) we won't set explicit dispatcher for the last candidate on the
queue effectively allowing a control call like callsame/nextsame to
obtain routine's default dispatcher. This fixes a situation when a
wrapped method cannot pass control over to the next method on
MethodDispatcher queue.

rakudo#2178
vrurg added a commit to vrurg/rakudo that referenced this issue Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant