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

Fix recent JVM breakage (Rakudo::Unicodey) #4290

Closed
wants to merge 3 commits into from

Conversation

usev6
Copy link
Contributor

@usev6 usev6 commented Apr 2, 2021

No description provided.

This looks like an oversight from 817d113. The old code
augmented class Str which had and attribute $!value -- but the
new class Rakudo::Unicodey does not.
We can't even refer to NFC and friends as a return type, since they
are completely unknown.
For unknown reasons using native arrays in the core doesn't work.
It looks like the array is created as VMArray and not VMArray_i.
The resulting error is (in a couple of spectest files):

  VMArray representation does not implement push_native

I'd say we are still in the 'make it correct' phase for the JVM
backend, so it doesn't matter if the code runs a bit slower (by
not using NQP ops).
@usev6 usev6 requested a review from lizmat April 2, 2021 19:50
@lizmat
Copy link
Contributor

lizmat commented Apr 2, 2021

So not even an my int @a works in the setting on the JVM backend? Even that late in the setting?

lizmat added a commit that referenced this pull request Apr 2, 2021
@lizmat
Copy link
Contributor

lizmat commented Apr 2, 2021

I've just pushed 496f986e6c in the hope it will stick on the JVM backend. If it doesn't, let me know, and I'll make it use an untyped array. Thing is that I will need do make all of the nqp::shift_i occurrences backend specific :-(

@lizmat lizmat marked this pull request as draft April 2, 2021 22:42
@usev6
Copy link
Contributor Author

usev6 commented Apr 3, 2021

So not even an my int @A works in the setting on the JVM backend? Even that late in the setting?

I'm afraid that is the case. As far as I can see native arrays are mis-compiled into non-native arrays in the setting.

So, after 496f986e6c the JVM backend builds, but method ords for strings does not work:

$ ./rakudo-j -e 'dd "foo".ords'
VMArray representation does not implement push_native
  in block <unit> at -e line 1

Thing is that I will need do make all of the nqp::shift_i occurrences backend specific :-(

Do you refer to upcoming changes? Because for now things seem to work if an untyped array is used (as in 6014cfa).

$ ./rakudo-j -e 'dd "foo".ords'  ## with an untyped array and without nqp::push_i
(102, 111, 111).Seq

@lizmat
Copy link
Contributor

lizmat commented Apr 3, 2021

Hopefully fixed with b88e1ca3bf Please close this PR if so :-) Thanks for the nudging and the PR!

@usev6
Copy link
Contributor Author

usev6 commented Apr 3, 2021

It looks like we finally found a solution: #4293 -- and it doesn't even require many quirks.

The main discovery for me is that

my int @array;

is miscompiled in the setting (on the JVM backend, that is), while

my @array := array[int].new;

seems to work. That's good to know.

I'm closing this PR.

@usev6 usev6 closed this Apr 3, 2021
@usev6 usev6 deleted the jvm_unicodey branch April 3, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants