Here are my preliminary findings about this problem.
I used the following evaluation for my debugging:
$ ./perl6-j -e 'say (so 1).perl'
1
The optimizer generates a QAST::Want with two children
(output generated with RAKUDO_OPTIMIZER_DEBUG=1):
[...]
- QAST::Op(callstatic &say) <sunk> :statement_id<?> say (so 1).perl
- QAST::Op(hllize) <wanted>
- QAST::Op(callmethod perl) <> perl
- QAST::Want <>
- QAST::WVal(Bool) <>
- Ii
- QAST::IVal(1) <>
I tried the following patch (skipping the creation of that
QAST::Want if $ret_value is an instance of a Bool) and all
the tests mentioned in my original post passed afterward:
====
diff --git a/src/Perl6/Optimizer.nqp b/src/Perl6/Optimizer.nqp
index 12398ba..dcdd55b 100644
--- a/src/Perl6/Optimizer.nqp
+++ b/src/Perl6/Optimizer.nqp
@@ -1435,7 +1435,7 @@ class Perl6::Optimizer {
my $want;
if !nqp::isconcrete($ret_value) {
# can we create a Want with a type object??? XXX
- } elsif nqp::istype($ret_value,
$!symbols.find_in_setting("Int")) && !nqp::isbig_I(nqp::decont($ret_value)) {
+ } elsif nqp::istype($ret_value,
$!symbols.find_in_setting("Int")) && !nqp::istype($ret_value,
$!symbols.find_in_setting("Bool")) && !nqp::isbig_I(nqp::decont($ret_value)) {
$want := QAST::Want.new($wval,
"Ii",
QAST::IVal.new(:value(nqp::unbox_i($ret_value))));
} elsif nqp::istype($ret_value,
$!symbols.find_in_setting("Num")) {
====
With that patch the evaluation gave:
$ ./perl6-j -e 'say (so 1).perl'
Bool::True
The generated QAST from the optimizer didn't had the Want
anymore (as expected):
[...]
- QAST::Op(callstatic &say) <sunk> :statement_id<?> say (so 1).perl
- QAST::Op(hllize) <wanted>
- QAST::Op(callmethod perl) <> perl
- QAST::WVal(Bool) <>
In src/Perl6/Optimizer.nqp there is an comment that made me think
that those QAST::Want were only meant for instances of Int, Num
or Str:
https://github.com/rakudo/rakudo/blob/nom/src/Perl6/Optimizer.nqp#L1433-L1434
We also generate those QAST::Want for Bool or Mixins like here:
$ ./perl6-j -e 'my $value = 42 but False; say ?$value'
True
[...]
- QAST::Want <>
- QAST::WVal(Int+{<anon|1285927979>}) <>
- Ii
- QAST::IVal(42) <>
I asked about my patch on #perl6-dev. There was no clear result,
but the tenor of the discussion was, that the JVM backend should
be able to interprete those QAST::Want correctly.
==== start of discussion on IRC -- cmp.
http://irclog.perlgeek.de/perl6-dev/2016-10-02#i_13323463
bartolin could someone take a look at
https://github.com/usev6/rakudo/commit/a547db7ebe ? It's an attempt to fix RT
#129782 but I'm not sure if there is indeed a (kind of) bug in the optimizer or
if the fix has to happen in the JVM specific code.
synopsebot6 Link: https://rt.perl.org/rt3//Public/Bug/Display.html?id=129782
MasterDuke bartolin: purely nitpicking, but if you put the added conditional
at the end, the diff would be a little easier to read
MasterDuke (not that one line is all that difficult...)
bartolin MasterDuke: thanks, noted. I wondered where the additional check
should be placed wrt performance, since the order could make a difference
performance wise, couldn't it? But, I've no idea how hot that path is
MasterDuke yeah, i was thinking about that after i said it
MasterDuke since i would definitely prioritize almost any performance
difference over the minor change in readability of a one-line commit
* bartolin nods
psch r: sub f(--> Bool) { so 1 }; say f().WHAT
camelia rakudo-jvm 2a1605, rakudo-moar 4abc28: OUTPUT«(Bool)»
psch r: sub f() { so 1 }; say f().WHAT
camelia rakudo-jvm 2a1605, rakudo-moar 4abc28: OUTPUT«(Bool)»
psch r: sub f() { so 1 }; say f().perl
camelia rakudo-jvm 2a1605, rakudo-moar 4abc28: OUTPUT«Bool::True»
psch r: say (so 1).perl
camelia rakudo-jvm 2a1605: OUTPUT«1»
camelia ..rakudo-moar 4abc28: OUTPUT«Bool::True»
psch that is honestly weird
psch bartolin: i'm inclined to think that the QAST::Want behavior is the
actual problem though
psch r: sub f { sub { say 1 } }; f()() # i am somehow reminded of this
camelia rakudo-moar 4abc28: OUTPUT«1»
camelia ..rakudo-jvm 2a1605: ( no output )
psch aw don't be shy camelia, show the NPE :P
psch r: sub f { sub { say 1 } }; f()() # i
camelia rakudo-moar 4abc28: OUTPUT«1»
camelia ..rakudo-jvm 2a1605: OUTPUT«java.lang.NullPointerException in
block <unit> at <tmp> line 1»
bartolin psch: so, you would say it makes sense to have a QAST::Want with a
QAST::WVal for the 'True' and a QAST::IVal for the '1'?
bartolin psch: ... and the interpretation of that QAST::Want on JVM is wrong?
psch bartolin: well, moar inlines it to Want <> with WVal(Bool) and
IVal(1) as children
psch bartolin: the &so call, that is
bartolin psch: oh, your last evaluation does not give a NPE with
--optimize=off -- I didn't know that
bartolin psch: yeah, the QAST is the same on JVM (if I'm not mistaken)
psch bartolin: and considering the post-optimize QAST looks the same
across the backends but doesn't give the same result... well there's probably
something wrong in the wrong backend :)
psch bartolin: i'd guess it's probably something with how we have to do
returns out of band on jvm and sometimes don't carry the right $*WANT around or
so
psch bartolin: but honestly those bits of the QAST -> JAST Compiler
aren't particularly transparent to me :)
bartolin psch: from the comments
(https://github.com/rakudo/rakudo/blob/nom/src/Perl6/Optimizer.nqp#L1433) I got
the impression that those QAST::Want are really only meant for Int, Num and Str
(in order to add a native variant) -- and not for Bool
bartolin timotimo: maybe you can shed some light? (wrt
https://github.com/usev6/rakudo/commit/a547db7ebe)
timotimo whoops, that want is totally broken :)
timotimo it should have the Ii and the IVal at the same level as the
WVal(Bool)
bartolin timotimo: sorry, that was my fault
timotimo oh, phew
bartolin fixed
timotimo your patch may be right
bartolin r: my $value = 42 but False; say ?$value
camelia rakudo-jvm 2a1605: OUTPUT«True»
camelia ..rakudo-moar 28c23a: OUTPUT«False»
timotimo o_O
bartolin r-j gives False with --optimize=off here
timotimo ugh ;(
bartolin I guess it's the same problem (according to the output generated
with RAKUDO_OPTIMIZER_DEBUG=1)
timotimo might be, yeah :\
bartolin so, if my patch makes sense, it is not complete
psch bartolin: well, Bool is Int on one hand
psch bartolin: and on the other, there's demonstrably other cases where
the optimizer produces something related to Wants that works on moar and
doesn't on jvm
psch bartolin: so i think the optimizer isn't to blame
bartolin psch: yeah. but do we really need the Want there? I mean, both the
optimizer and the JVM backend could be wrong :-)
psch m: sub f(int $) { say "yup" }; f Bool
camelia rakudo-moar 28c23a: OUTPUT«Cannot unbox a type object in sub f at
<tmp> line 1 in block <unit> at <tmp> line 1»
psch m: sub f(int $) { say "yup" }; f True
camelia rakudo-moar 28c23a: OUTPUT«yup»
psch j: sub f(int $) { say "yup" }; f True
camelia rakudo-jvm 2a1605: OUTPUT«yup»
psch bartolin: afaiu the Want means "here's the native value in case you
need it", and an IVal makes perfect sense for in the case of Bool
bartolin psch: ok, at least I understood the meaning of the Want, then :-)
timotimo well, something is mishandling that case, though
psch yeah, the Want handling in JAST::Compiler, probably :S
psch but that seems to be complicated stuff and i don't get it
timotimo right, the jast compiler isn't easy
bartolin ok, so I won't open a PR then, but will add the discussion to the
ticket
==== end of discussion on IRC -- powered by
https://github.com/usev6/dump-irc-logs
