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

[JVM] Creation of Bag objects broken #4272

Closed
usev6 opened this issue Mar 28, 2021 · 2 comments
Closed

[JVM] Creation of Bag objects broken #4272

usev6 opened this issue Mar 28, 2021 · 2 comments
Assignees
Labels
JVM Related to Rakudo-JVM

Comments

@usev6
Copy link
Contributor

usev6 commented Mar 28, 2021

The Problem

A lot of spectests for Bag fail on the JVM backend.

The breakage can be seen with a simple command (which should print Bag(42)):

$ ./rakudo-j -e 'say bag(42)'
java.lang.RuntimeException: No such attribute '$!value' for this object
  in block <unit> at -e line 1
$ ./rakudo-j --ll-exception -e 'say bag(42)'
java.lang.RuntimeException: No such attribute '$!value' for this object
  in ADD-ITERATOR-TO-BAG (gen/jvm/CORE.c.setting:10734)
  in create-from-iterator (gen/jvm/CORE.c.setting:60275)
  in dispatch:<!> (gen/jvm/CORE.c.setting:2083)
  in new (gen/jvm/CORE.c.setting:60288)
  in new (gen/jvm/CORE.c.setting:1175)
  in bag (gen/jvm/CORE.c.setting:61844)
  in <unit> (-e:1)
  in <unit-outer> (-e:1)
  in eval (gen/jvm/stage2/NQPHLL.nqp:1216)
  in <anon> (gen/jvm/stage2/NQPHLL.nqp:1328)
  in command_eval (gen/jvm/stage2/NQPHLL.nqp:1325)
  in command_eval (gen/jvm/Compiler.nqp:119)
  in command_line (gen/jvm/stage2/NQPHLL.nqp:1309)
  in MAIN (gen/jvm/rakudo.nqp:133)
  in <mainline> (gen/jvm/rakudo.nqp:121)
  in <anon> (gen/jvm/rakudo.nqp)

The problem was introduced a couple of days agow with 7c0a556e9c. With that commit reverted things work as expected.

I think the following code is a golf of the underlying problem:

$ ./rakudo-m -e 'use nqp; my $pair := nqp::create(Pair); nqp::bindattr($pair,Pair,"\$!value",nqp::add_i(nqp::getattr(($pair := Pair.new(42,0)),Pair, "\$!value"),1)); say $pair'
42 => 1
$ ./rakudo-j -e 'use nqp; my $pair := nqp::create(Pair); nqp::bindattr($pair,Pair,"\$!value",nqp::add_i(nqp::getattr(($pair := Pair.new(42,0)),Pair,"\$!value"),1)); say $pair'
42 => 0

Please note, that I'm using my $pair := nqp::create(Pair); instead of just my $pair; as it is done in https://github.com/rakudo/rakudo/blob/776f1a626d/src/core.c/Rakudo/QuantHash.pm6#L579
The latter fails with the failure mode from the original example:

$ ./rakudo-j -e 'use nqp; my $pair; nqp::bindattr($pair,Pair,"\$!value",nqp::add_i(nqp::getattr(($pair := Pair.new(42,0)),Pair,"\$!value"),1)); say $pair'
java.lang.RuntimeException: No such attribute '$!value' for this object
  in block <unit> at -e line 1

So AFAICS, the order of execution differs between the two backends. On MoarVM the binding to $pair (within nqp::getattr) seems to be executed first and the ǹqp::bindattr runs afterward -- overwriting $!value with 1.
On the JVM the nqp::bindattr seems to be executed first and the binding to $pair (within nqp::getattr) overwrites ``$!valuewith0```.

Environment

  • Compiler version (perl6 -v or raku -v): Welcome to Rakudo(tm) v2021.03-12-gb4367a8f9.
    Implementing the Raku(tm) programming language v6.d.
    Built on JVM.
@usev6 usev6 added the JVM Related to Rakudo-JVM label Mar 28, 2021
usev6 referenced this issue Mar 28, 2021
For the case where keys already exist in the Bag.  This same optimization
(using nqp::ifnull(nqp::atkey instead of nqp::if(nqp::existskey...nqp::atkey)
could probably be done for *all* the methods in Rakudo/QuantHash

Spotted by MasterDuke++
@lizmat lizmat self-assigned this Mar 28, 2021
@lizmat lizmat closed this as completed in dc50951 Mar 28, 2021
@lizmat lizmat reopened this Mar 28, 2021
@lizmat
Copy link
Contributor

lizmat commented Mar 28, 2021

Oops, I guess Github being a little overzealous. I think it fixes the issue on the JVM backend, but am not 100% sure as no way to test.

@usev6
Copy link
Contributor Author

usev6 commented Mar 29, 2021

Spectest looks good again. \o/
I'm closing this issue.

@usev6 usev6 closed this as completed Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JVM Related to Rakudo-JVM
Projects
None yet
Development

No branches or pull requests

2 participants