`is rw` candidates get called even if a non-rw argument is passed

[p6rt]

Migrated from rt.perl.org#129812 (status was 'open')

Searchable as RT129812$

[p6rt]

From @zoffixznet

This code shows the bug​:

  zoffix@​leliana​:\~$ perl6 -e 'm​: multi foo ($) {"right" }; multi
foo ($ is rw) {"wrong"}; say foo "42"'   wrong

And if we turn off the optimizer, we get the right candidate called
(same would happen if we add more complex sub bodies, so possibly the
sub gets inlined)​:

  zoffix@​leliana​:\~$ perl6 --optimize=off -e 'm​: multi foo ($)
{"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"'   right

[p6rt]

From @dogbert17

On Wed, 05 Oct 2016 14​:23​:57 -0700, cpan@​zoffix.com wrote​:

This code shows the bug​:

 zoffix@​leliana​:\~$ perl6 \-e 'm​: multi foo \($\) \{"right" \}; multi  

foo ($ is rw) {"wrong"}; say foo "42"' wrong

And if we turn off the optimizer, we get the right candidate called
(same would happen if we add more complex sub bodies, so possibly the
sub gets inlined)​:

 zoffix@​leliana​:\~$ perl6 \-\-optimize=off \-e 'm​: multi foo \($\)  

{"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"' right

It would seem that this bug was fixed with https://github.com/rakudo/rakudo/commit/f8b3469439108fead043bab2bd27bde4bac50dca Test(s) needed

[p6rt]

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

[p6rt]

From @skids

On Sat, 05 Aug 2017 06​:28​:34 -0700, jan-olof.hendig@​bredband.net wrote​:

On Wed, 05 Oct 2016 14​:23​:57 -0700, cpan@​zoffix.com wrote​:

This code shows the bug​:

zoffix@​leliana​:\~$ perl6 -e 'm​: multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"' wrong

And if we turn off the optimizer, we get the right candidate called (same would happen if we add more complex sub bodies, so possibly the sub gets inlined)​:

zoffix@​leliana​:\~$ perl6 --optimize=off -e 'm​: multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"' right

It would seem that this bug was fixed with https://github.com/rakudo/rakudo/commit/f8b3469439108fead043bab2bd27bde4bac50dca Test(s) needed

Tests added with roast commit 63181b3c9. So resolving.

[p6rt]

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

[p6rt]

From @usev6

On Thu, 21 Sep 2017 16​:27​:09 -0700, bri@​abrij.org wrote​:

On Sat, 05 Aug 2017 06​:28​:34 -0700, jan-olof.hendig@​bredband.net wrote​:

On Wed, 05 Oct 2016 14​:23​:57 -0700, cpan@​zoffix.com wrote​:

This code shows the bug​:

zoffix@​leliana​:\~$ perl6 -e 'm​: multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"' wrong

And if we turn off the optimizer, we get the right candidate called (same would happen if we add more complex sub bodies, so possibly the sub gets inlined)​:

zoffix@​leliana​:\~$ perl6 --optimize=off -e 'm​: multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"' right

It would seem that this bug was fixed with https://github.com/rakudo/rakudo/commit/f8b3469439108fead043bab2bd27bde4bac50dca Test(s) needed

Tests added with roast commit 63181b3c9. So resolving.

This bug is still present on the JVM backend (and the test code is passing with optimize=1). I'm going to reopen this ticket and tag it with [JVM].

I tried to find the source of the bug and I think it is in 'analyze_dispatch' in src/Perl6/Metamodel/BOOTSTRAP.nqp. We only check for literals passing to rw parameters if the argument passed is native. I added a similar check for the case with a non-native argument and the problem disappeared​: https://github.com/usev6/rakudo/commit/db077336c5

I'm not sure if that's the right way to fix the problem -- especially given that some tests that expect code like '1++' to fail with X​::Multi​::NoMatch are failing now. The new exception is X​::TypeCheck​::Argument+{X​::Comp} -- which makes some sense as well.

I'd appreciate some feedback on this topic.

Thanks

Christian

[p6rt]

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

[p6rt]

From @usev6

This is still a problem on the JVM backend. I tried a second time to find the underlying problem and arrived at the same conclusion​: There seems to be something wrong in 'analyze_dispatch'. When running the given code

  multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"

'analyze_dispatch' returns the second sub with $MD_CT_DECIDED. This (wrong) result is taken for real on the JVM backend, whereas on MoarVM it isn't really used in this case -- cmp. https://github.com/rakudo/rakudo/blob/4df02facd0/src/Perl6/Optimizer.nqp#L3109-L3113

I didn't grasp all the details, but the problem might be related to the fact that 'sort_dispatchees_internal' returns an array with five results​: [2nd_sub, Mu, 1st_sub, Mu, Mu]. The second sub comes first, because it is narrower than the first sub. The Mu at index 1 seems to indicate the end of a tied group. This leads to 'analyze_dispatch' looking at the second sub first, not detecting a problem there (due to the missing check for a literal being passed in) and returning this sub after seeing the Mu.

I still think my patch from 2017 makes sense. With this patch, 'analyze_dispatch' rejects the second sub, notices that it didn't analyze all candidates and returns $MD_CT_NOT_SURE. But it would be even better if 'analyze_dispatch' dispatches to the first sub with $MD_CT_DECIDED.

[p6rt]

From @usev6

This is still a problem on the JVM backend. I tried a second time to find the underlying problem and arrived at the same conclusion​: There seems to be something wrong in 'analyze_dispatch'. When running the given code

  multi foo ($) {"right" }; multi foo ($ is rw) {"wrong"}; say foo "42"

'analyze_dispatch' returns the second sub with $MD_CT_DECIDED. This (wrong) result is taken for real on the JVM backend, whereas on MoarVM it isn't really used in this case -- cmp. https://github.com/rakudo/rakudo/blob/4df02facd0/src/Perl6/Optimizer.nqp#L3109-L3113

I didn't grasp all the details, but the problem might be related to the fact that 'sort_dispatchees_internal' returns an array with five results​: [2nd_sub, Mu, 1st_sub, Mu, Mu]. The second sub comes first, because it is narrower than the first sub. The Mu at index 1 seems to indicate the end of a tied group. This leads to 'analyze_dispatch' looking at the second sub first, not detecting a problem there (due to the missing check for a literal being passed in) and returning this sub after seeing the Mu.

I still think my patch from 2017 makes sense. With this patch, 'analyze_dispatch' rejects the second sub, notices that it didn't analyze all candidates and returns $MD_CT_NOT_SURE. But it would be even better if 'analyze_dispatch' dispatches to the first sub with $MD_CT_DECIDED.

[p6rt]

From @usev6

I forgot to add​: 'find_best_dispatchee' does a better job and recognizes that the second sub won't work with a literal​: https://github.com/rakudo/rakudo/blob/4df02facd0/src/Perl6/bootstrap.c/BOOTSTRAP.nqp#L2665-L2668

This code is executed on MoarVM -- but on the JVM we depend on the compile time analysis being correct.

[usev6]

It looks like I actually committed the patch the fixes the remaining problem for the JVM backend, but didn't close this issue. Doing that now.

• Rakudo fix: https://github.com/rakudo/rakudo/commit/12d4514a3d
• Roast cleanup (remove fudging): https://github.com/Raku/roast/commit/3f2cb17c5b