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] Improve handling of unicode numbers #779

Merged
merged 2 commits into from Oct 2, 2022

Conversation

usev6
Copy link
Contributor

@usev6 usev6 commented Sep 19, 2022

No description provided.

This is everything but a proper implementation of getuniprop_str and
unipropcode. But it makes the evaluations mentioned in
rakudo/rakudo#4760 work on the JVM backend.
@usev6 usev6 marked this pull request as ready for review September 20, 2022 05:44
@usev6
Copy link
Contributor Author

usev6 commented Sep 20, 2022

I didn't see any additional spectest breakage from this changes.

@usev6
Copy link
Contributor Author

usev6 commented Oct 2, 2022

I'm tempted to merge this even without getting another pair of eyes.

Sure, the addition to unipropcode and getuniprop_str look quite ad hoc and are nothing in comparison to what MoarVM does there. (Even though I picked the same numbers as MoarVM for UNIPROP_NUMERIC_VALUE_NUMERATOR and UNIPROP_NUMERIC_VALUE_DENOMINATOR.)

But on the other hand the current situation (described in rakudo/rakudo#4760) is clearly LTA. The usage of nqp::getuniprop_str in Rakudo's method numish (in https://github.com/rakudo/rakudo/blob/30a9d3a40a/src/Perl6/Actions.nqp#L8526-L8527) is quite central -- and the JVM backend can do better there. I'd hope the small patch wouldn't hinder any future improvements in that area.

@usev6
Copy link
Contributor Author

usev6 commented Oct 2, 2022

@lizmat thanks for looking!

@usev6 usev6 merged commit bddcbca into Raku:master Oct 2, 2022
@usev6 usev6 deleted the jvm_unicode_numbers branch October 2, 2022 19:57
usev6 added a commit to Raku/roast that referenced this pull request Oct 8, 2022
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.

None yet

2 participants