[perl #129812] [OPTIMIZER] `is rw` candidates get called even if a non-rw argument is passed

[Zoffix Znet]

# New Ticket Created by Zoffix Znet
# Please include the string: [perl #129812]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org/Ticket/Display.html?id=129812 >



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

[Jan-Olof Hendig via RT]

On Wed, 05 Oct 2016 14:23:57 -0700, cp...@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

[Brian S. Julin via RT]

Tests added with roast commit 63181b3c9. So resolving.

[Christian Bartolomaeus via RT]

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

[Christian Bartolomaeus via RT]

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.

[Christian Bartolomaeus via RT]

This now works on the JVM backend, too. Fixed with https://github.com/rakudo/rakudo/commit/12d4514a3d

I'm closing this ticket as 'resolved'.