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

multisub dispatch broken in roles #3018

Closed
p6rt opened this issue Jan 2, 2013 · 5 comments
Closed

multisub dispatch broken in roles #3018

p6rt opened this issue Jan 2, 2013 · 5 comments

Comments

@p6rt
Copy link

p6rt commented Jan 2, 2013

Migrated from rt.perl.org#116268 (status was 'resolved')

Searchable as RT116268$

@p6rt
Copy link
Author

p6rt commented Jan 2, 2013

From doy@tozt.net

< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; role Blorg { method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«Too many positional parameters passed; got 1 but expected 0␤ in sub bar at /tmp/5CaNa5PWz1​:1␤ in method
  do_stuff at /tmp/5CaNa5PWz1​:1␤ in block at /tmp/5CaNa5PWz1​:1␤␤»
< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; class Blorg { method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«BAZ␤»
< doy> multisub dispatch doesn't appear to work properly in roles for some reason?

@p6rt
Copy link
Author

p6rt commented Jan 2, 2013

From @jnthn

On Wed Jan 02 07​:11​:02 2013, doy@​tozt.net wrote​:

< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; role Blorg {
method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«Too many positional parameters passed;
got 1 but expected 0␤ in sub bar at /tmp/5CaNa5PWz1​:1␤ in method
do_stuff at /tmp/5CaNa5PWz1​:1␤ in block at
/tmp/5CaNa5PWz1​:1␤␤»

This is actually the correct output. A proto's signature must be at
least as wide as those of its candidates if its candidates are to be
reachable. You probably want something like like​:

proto bar(|) {*}

Or to restrict to a single arg​:

proto bar($) {*}

< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; class Blorg {
method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«BAZ␤»

So the real bug is that this one works out. Turns out that was because
the optimizer was not checking the proto signature properly and so
inlining the multi dispatch when it should not have done so.

I've now fixed this analysis bug in the optimizer, and it correctly
points out that the dispatch cannot possibly work at compile now for
both cases.

Note that the reason the role one did detect the error is that the
optimizer doesn't currently do code transformations inside of roles, so
it didn't do the inlining and thus went through the normal path and that
caught the problem.

Tagging testneeded.

Thanks,

Jonathan

@p6rt
Copy link
Author

p6rt commented Jan 2, 2013

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Jan 19, 2014

From @coke

On Wed Jan 02 09​:24​:55 2013, jnthn@​jnthn.net wrote​:

On Wed Jan 02 07​:11​:02 2013, doy@​tozt.net wrote​:

< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; role Blorg {
method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«Too many positional parameters passed;
got 1 but expected 0␤ in sub bar at /tmp/5CaNa5PWz1​:1␤ in method
do_stuff at /tmp/5CaNa5PWz1​:1␤ in block at
/tmp/5CaNa5PWz1​:1␤␤»

This is actually the correct output. A proto's signature must be at
least as wide as those of its candidates if its candidates are to be
reachable. You probably want something like like​:

proto bar(|) {*}

Or to restrict to a single arg​:

proto bar($) {*}

< doy> r​: proto bar {*}; multi bar ($baz) { "BAZ" }; class Blorg {
method do_stuff { bar "baz" } }; say Blorg.new.do_stuff
<+p6eval> rakudo 13e805​: OUTPUT«BAZ␤»

So the real bug is that this one works out. Turns out that was because
the optimizer was not checking the proto signature properly and so
inlining the multi dispatch when it should not have done so.

I've now fixed this analysis bug in the optimizer, and it correctly
points out that the dispatch cannot possibly work at compile now for
both cases.

Note that the reason the role one did detect the error is that the
optimizer doesn't currently do code transformations inside of roles, so
it didn't do the inlining and thus went through the normal path and that
caught the problem.

Tagging testneeded.

Thanks,

Jonathan

Test added to integration/weird-errors.t for lack of a better file.

--
Will "Coke" Coleda

@p6rt
Copy link
Author

p6rt commented Jan 19, 2014

@coke - Status changed from 'open' to 'resolved'

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

1 participant