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

Different implementation for nqp::substr for Moar and [JVM] (negative offset) #324

Open
usev6 opened this issue Nov 28, 2016 · 1 comment

Comments

@usev6
Copy link
Contributor

usev6 commented Nov 28, 2016

The implementation of nqp::substr differs between Moar and JVM with regard to negative offsets:

bartolin    r: use nqp; say nqp::substr("bar", -1, 1)
camelia     rakudo-moar 054aca: OUTPUT«r␤»
camelia     ..rakudo-jvm 76b061: OUTPUT«java.lang.StringIndexOutOfBoundsException: String index out of range: -1␤  in block <unit> at <tmp> line 1␤␤»

For Moar there is a comment stating explicitely that negative offsets count from the end: https://github.com/MoarVM/MoarVM/blob/ee0b54e264ca3647f55ffd115238b72c7a0729f5/src/strings/ops.c#L290

What follows is some additional discussion from #perl6-dev:

AlexDaniel m: use nqp; say nqp::substr("bar", -2, 1)
camelia rakudo-moar 054aca: OUTPUT«a␤»
AlexDaniel m: use nqp; say nqp::substr("bar", -4, 1)
camelia rakudo-moar 054aca: OUTPUT«␤»
AlexDaniel m: use nqp; say nqp::substr("bar", -5, 1)
camelia rakudo-moar 054aca: OUTPUT«Substring end (-1) cannot be less than 0␤ in block at line 1␤␤»
AlexDaniel m: use nqp; say nqp::substr("bar", -4, 5)
camelia rakudo-moar 054aca: OUTPUT«bar␤»
camelia rakudo-moar 054aca: OUTPUT«Start argument to substr out of range. Is: -1, should be in 0..3; use *-1 if you want to index relative to the end␤ in block at line 1␤␤Actually thrown at:␤ in block at line 1␤␤»
FROGGS bartolin: yes, please open said ticket
bartolin will do
perlpilot bartolin: be sure to include a transcript of what AlexDaniel did above ... even if I can accept that it should handle negative offsets, that second to last one is just crazy

I've stumbled above this problem when I looked a failing test for rakudo-j (from S04-statements/terminator.t):

bartolin    r: {my $x = 2;
camelia     rakudo-jvm 76b061: OUTPUT«===SORRY!===␤java.lang.StringIndexOutOfBoundsException: String index 
                out of range: -1␤»
camelia     ..rakudo-moar 054aca: OUTPUT«===SORRY!=== Error while compiling <tmp>␤Missing block␤at 
                <tmp>:1␤------> {my $x = 2;⏏<EOL>␤» 

The relevant code in Rakudo's src/Perl6/Grammar.nqp (https://github.com/rakudo/rakudo/blob/4dffef7a7ab035da03803d4a85dc3ba7cccb3647/src/Perl6/Grammar.nqp#L285) is supposed to examine the preceding character -- but it calls nqp::substring with a offset of -1. (I'll open a PR for Rakudo for this specific case.)

usev6 referenced this issue in rakudo/rakudo Apr 26, 2019
This is only a quick workaround to make rakudo-j usable for the
release.

The old code led to a StringIndexOutOfBoundsException, because
nqp::substr was called with -1 as the third positional
parameter ($length). On MoarVM this seems to mean "go to the
end of the string" -- cmp.
https://github.com/MoarVM/MoarVM/blob/8eb232dbcc/src/strings/ops.c#L716-L718

A cleaner fix would probably be to let nqp::substr work similar
on the JVM backend. But that's for after the release.
@usev6
Copy link
Contributor Author

usev6 commented Apr 27, 2019

I've opened a second, more general issue for the implementation of nqp::substr: #533. Nevertheless, I'll leave this issue open for now.

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

No branches or pull requests

1 participant