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

`Cannot call method 'sink' on a null object` when using `loop` in sink context #2069

Open
AlexDaniel opened this Issue Jul 14, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@AlexDaniel
Member

AlexDaniel commented Jul 14, 2018

Golf:

sub foo {
    loop (my int $i = 0; $i <= 2; $i++) {
        note hello
    }
}

for 42 {
    foo
}

This was dug out of toaster. The issue was in Native::Packing module and it required this “fix”: p6-pdf/Native-Packing-p6@23dcfc7. I think it's a rakudo bug and we should fix it in rakudo instead.

Bisected: (2018-07-09) 6d27166

Output on all releases: https://gist.github.com/2f1cbd34b2991d15c7dcda3a98c1e623

Note that the output was different in the past, and the fix was in (2016-08-05) 589061e.

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 14, 2018

Member

Another module that is failing is Slang::SQL and bisectable points to the same commit (2018-07-09) 6d27166. There's no golf for that module, but when the OP issue is fixed I guess one can simply try Slang::SQL's tests to see if it's fixed also.

When running its tests the output is:

===SORRY!===
Cannot call method 'ACCEPTS' on a null object
Member

AlexDaniel commented Jul 14, 2018

Another module that is failing is Slang::SQL and bisectable points to the same commit (2018-07-09) 6d27166. There's no golf for that module, but when the OP issue is fixed I guess one can simply try Slang::SQL's tests to see if it's fixed also.

When running its tests the output is:

===SORRY!===
Cannot call method 'ACCEPTS' on a null object

jnthn added a commit that referenced this issue Jul 15, 2018

Make all cases of statementlist `loop` eval to Nil
Previously, `loop { }` and `loop (; $++ < 3;) { }` would evaluate to
Nil, but `loop (my $i = 0; $i < 3; $i++) { }` would evaluate to an
`nqp::null` due to the oversight that is corrected in this commit. This
bug was made more visible when recent changes to return handling removed
the `nqp::null` mapping into Mu, resulting in the problem observed in
issue #2069. Since all other loop cases evaluated to Nil, this created a
discontinuity where one particular case of `loop` would instead evaluate
to Mu. Therefore, there was already a bug; the recent changes just made
it problematic in more cases.
@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 15, 2018

Member

The return value handling changes removed a check for if we somehow ended up with an nqp::null being returned, and turning it into a Mu instead. This situation should really never happen, and the check was thus papering over other bugs (in the best case, just the one the golf exposes).

In this particular case the mapping to Mu also created a discontinuity: a loop (my int $i = 0; $i <= 2; $i++) { } as the last statement in a sub would end up with the sub evaluating to Mu, whereas every other kind of loop - and even a loop of the form loop { } or loop (; $++ < 3;) { } - would end up with it evaluating to Nil.

I've fixed that in aff96ba. It's possible that we have some other constructs leaking an nqp::null, so testing of the modules in question to make sure they also were due to the loop code-gen bug would be very welcome. Even if they're all fine, a test should be added before this issue can be resolved.

Member

jnthn commented Jul 15, 2018

The return value handling changes removed a check for if we somehow ended up with an nqp::null being returned, and turning it into a Mu instead. This situation should really never happen, and the check was thus papering over other bugs (in the best case, just the one the golf exposes).

In this particular case the mapping to Mu also created a discontinuity: a loop (my int $i = 0; $i <= 2; $i++) { } as the last statement in a sub would end up with the sub evaluating to Mu, whereas every other kind of loop - and even a loop of the form loop { } or loop (; $++ < 3;) { } - would end up with it evaluating to Nil.

I've fixed that in aff96ba. It's possible that we have some other constructs leaking an nqp::null, so testing of the modules in question to make sure they also were due to the loop code-gen bug would be very welcome. Even if they're all fine, a test should be added before this issue can be resolved.

@jnthn jnthn added the testneeded label Jul 15, 2018

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 16, 2018

Member

@jnthn Native::Packing is now OK but Slang::SQL is still failing with:

===SORRY!===
Cannot call method 'ACCEPTS' on a null object

Should I open another ticket?

Member

AlexDaniel commented Jul 16, 2018

@jnthn Native::Packing is now OK but Slang::SQL is still failing with:

===SORRY!===
Cannot call method 'ACCEPTS' on a null object

Should I open another ticket?

@jnthn

This comment has been minimized.

Show comment
Hide comment
@jnthn

jnthn Jul 16, 2018

Member

@AlexDaniel Yes, let's have a separate ticket for that, if nothing else to keep the regression tests we need to write straight.

Member

jnthn commented Jul 16, 2018

@AlexDaniel Yes, let's have a separate ticket for that, if nothing else to keep the regression tests we need to write straight.

@AlexDaniel

This comment has been minimized.

Show comment
Hide comment
@AlexDaniel

AlexDaniel Jul 16, 2018

Member

Cool. See #2069.

Member

AlexDaniel commented Jul 16, 2018

Cool. See #2069.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment