[perl #22214] set Nx, Sx with non-numeric Sx results in number

[Clinton A. Pierce]

# New Ticket Created by "Clinton A. Pierce"
# Please include the string: [perl #22214]
# in the subject line of all future correspondence about this issue.
# <URL: http://rt.perl.org/rt2/Ticket/Display.html?id=22214 >

Converting anything that contains a number to a Nx register causes the
register to recive that numeric value. Test case:

set S1, "Not really a number 2"
set N0, S1
print N0
end

Results in "2.00000". This probably isn't desirable behavior. I'd expect
"0.00000", personally. My viewpoint may be wrong.

[Luke Palmer]

I believe your intuition is correct. Here's a patch that makes string
to number conversion more perl5-like.

Luke

Index: string.c
===================================================================
RCS file: /cvs/public/parrot/string.c,v
retrieving revision 1.128
diff -U3 -r1.128 string.c
--- string.c 14 May 2003 08:05:57 -0000 1.128
+++ string.c 16 May 2003 01:01:37 -0000
@@ -1117,6 +1117,39 @@
INTVAL fake_exponent = 0;

while (start < end) {
+ int breakit = 0;
+ UINTVAL c = s->encoding->decode(start);
+ if (c == '-') {
+ sign = -1;
+ breakit = 1;
+ }
+ else if (c == '+') {
+ // sign is already 1
+ breakit = 1;
+ }
+ else if (c == '.') {
+ seen_dot = 1;
+ breakit = 1;
+ }
+ else if (isspace(c)) { // XXX: (LP) Is there some unicode
+ // whitespace routine?
+ // just advance
+ }
+ else if (isdigit(c)) { // XXX: (LP) Same here
+ break;
+ }
+ else {
+ return 0.0;
+ }
+
+ start = s->encoding->skip_forward(start, 1);
+
+ if (breakit) {
+ break;
+ }
+ }
+
+ while (start < end) {
UINTVAL c = s->encoding->decode(start);

if (s->type->is_digit(c)) {
@@ -1133,19 +1166,6 @@
if (seen_dot) {
fake_exponent--;
}
- }
- }
- else if (!in_number) {
- /* we've not yet seen any digits */
- if (c == '-') { /* XXX: ascii */
- sign = -1;
- }
- else if (c == '.') { /* XXX: ascii */
- seen_dot = 1;
- }
- else {
- seen_dot = 0;
- sign = 1;
}
}
else {
Index: t/op/number.t
===================================================================
RCS file: /cvs/public/parrot/t/op/number.t,v
retrieving revision 1.28
diff -U3 -r1.28 number.t
--- t/op/number.t 28 Dec 2002 01:37:54 -0000 1.28
+++ t/op/number.t 16 May 2003 01:01:37 -0000
@@ -1007,13 +1007,19 @@
set S1, "12.0"
set S2, "-2.45"
set S3, "25e2"
- set S4, "Banana"
+ set S4, "Banana32"
+ set S5, ".Monkey6"
+ set S6, " 42.65"
+ set S7, "12Chimp"

set N0, S0
set N1, S1
set N2, S2
set N3, S3
set N4, S4
+ set N5, S5
+ set N6, S6
+ set N7, S7

print N0
print "\n"
@@ -1025,6 +1031,12 @@
print "\n"
print N4
print "\n"
+ print N5
+ print "\n"
+ print N6
+ print "\n"
+ print N7
+ print "\n"

end
CODE
@@ -1033,6 +1045,9 @@
-2.450000
2500.000000
0.000000
+0.000000
+42.650000
+12.000000
OUTPUT
1;

[Luke Palmer]

Silly me. Here's that patch without the C++ comments %-)

Luke

Index: string.c
===================================================================
RCS file: /cvs/public/parrot/string.c,v
retrieving revision 1.128
diff -U3 -r1.128 string.c
--- string.c 14 May 2003 08:05:57 -0000 1.128
+++ string.c 16 May 2003 01:01:37 -0000
@@ -1117,6 +1117,39 @@
INTVAL fake_exponent = 0;

while (start < end) {
+ int breakit = 0;
+ UINTVAL c = s->encoding->decode(start);
+ if (c == '-') {
+ sign = -1;
+ breakit = 1;
+ }
+ else if (c == '+') {

+ /* sign is already 1 */

+ breakit = 1;
+ }
+ else if (c == '.') {
+ seen_dot = 1;
+ breakit = 1;
+ }

+ else if (isspace(c)) { /* XXX: (LP) Is there some unicode
+ whitespace routine? */
+ /* just advance */
+ }
+ else if (isdigit(c)) { /* XXX: (LP) Same here */

[Benjamin Goldberg]

Luke Palmer wrote:
[snip]

> + UINTVAL c = s->encoding->decode(start);

[snip]

> + else if (isspace(c)) { // XXX: (LP) Is there some unicode
> + // whitespace routine?
> + // just advance

Remember that strings have both an encoding and a type. The type, which
is AFAIK independent of encoding, (currently) may be either unicode or
ascii.

Presumably, this should be something like:

else if( s->type->is_space(c) ) {

Alas, struct parrot_chartype_t does not (yet) contain such a member.

> + }
> + else if (isdigit(c)) { // XXX: (LP) Same here

And certainly, this can be:

else if( s->type->is_digit(c) ) {

Since such a member does exist.

> + break;
> + }
> + else {

--
$a=24;split//,240513;s/\B/ => /for@@=qw(ac ab bc ba cb ca
);{push(@b,$a),($a-=6)^=1 for 2..$a/6x--$|;print "$@[$a%6
]\n";((6<=($a-=6))?$a+=$_[$a%6]-$a%6:($a=pop @b))&&redo;}

[Nicholas Clark]

On Thu, May 15, 2003 at 08:35:39PM -0600, Luke Palmer wrote:
> Silly me. Here's that patch without the C++ comments %-)

This bug is still marked open:

http://rt.perl.org/rt2/Ticket/Display.html?id=22214

> RCS file: /cvs/public/parrot/t/op/number.t,v
> retrieving revision 1.28
> diff -U3 -r1.28 number.t
> --- t/op/number.t 28 Dec 2002 01:37:54 -0000 1.28
> +++ t/op/number.t 16 May 2003 01:01:37 -0000
> @@ -1007,13 +1007,19 @@
> set S1, "12.0"
> set S2, "-2.45"
> set S3, "25e2"
> - set S4, "Banana"
> + set S4, "Banana32"
> + set S5, ".Monkey6"
> + set S6, " 42.65"
> + set S7, "12Chimp"

I don't see this test added, so should this patch be applied?

Nicholas Clark

[Leopold Toetsch]

Nicholas Clark <ni...@ccl4.org> wrote:
> On Thu, May 15, 2003 at 08:35:39PM -0600, Luke Palmer wrote:
>> Silly me. Here's that patch without the C++ comments %-)

> This bug is still marked open:

> http://rt.perl.org/rt2/Ticket/Display.html?id=22214

> I don't see this test added, so should this patch be applied?

Yes. With the remarks WRT is_digit et al merged. The test can go in
anyway.

> Nicholas Clark

leo

[Nicholas Clark]

Adding just the test gives:

t/op/number............NOK 37# Failed test (t/op/number.t at line 1005)
# got: '1.000000
# 12.000000
# -2.450000
# 2500.000000
# 32.000000
# 6.000000
# 42.650000
# 12.000000
# '
# expected: '1.000000
# 12.000000
# -2.450000
# 2500.000000
# 0.000000
# 0.000000
# 42.650000
# 12.000000
# '

Which is S4 and S5 from this set:

set S0, "1"

set S1, "12.0"
set S2, "-2.45"
set S3, "25e2"

set S4, "Banana32"
set S5, ".Monkey6"
set S6, " 42.65"
set S7, "12Chimp"

ie currently "Banana32" is converting to 32 and .Monkey6" to 6
Is this what we want?

Adding the string.c changes causes other tests to fail.

Nicholas Clark