Skip to content

Commit 029e962

Browse files
committed
strings: make write_decimal handle runtime min_i64 without the min_i64 constant
Address the review on #27522: on the JS backend the min_i64 constant lowers incorrectly, so the 'if n == min_i64' guard was skipped and a genuine runtime min_i64 (e.g. from '-9223372036854775808'.i64()) fell into the digit loop where negating it overflows i64. Compute the magnitude with wrapping unsigned arithmetic (u64(0) - u64(n)) and delegate to write_u_decimal instead. This is parity-correct for runtime min_i64 on both backends, removes the dependence on the constant, and drops the allocating n.str() fallback so the C path is now allocation-free for every input.
1 parent 811733e commit 029e962

3 files changed

Lines changed: 18 additions & 37 deletions

File tree

vlib/strings/builder.c.v

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -74,32 +74,20 @@ pub fn (mut b Builder) write_byte(data u8) {
7474
// write_decimal appends a decimal representation of the number `n` into the builder `b`,
7575
// without dynamic allocation. The higher order digits come first, i.e. 6123 will be written
7676
// with the digit `6` first, then `1`, then `2` and `3` last.
77-
@[direct_array_access]
7877
pub fn (mut b Builder) write_decimal(n i64) {
7978
if n == 0 {
8079
b.write_u8(0x30)
8180
return
8281
}
83-
if n == min_i64 {
84-
b.write_string(n.str())
85-
return
86-
}
87-
88-
mut buf := [25]u8{}
89-
mut x := if n < 0 { -n } else { n }
90-
mut i := 24
91-
for x != 0 {
92-
nextx := x / 10
93-
r := x % 10
94-
buf[i] = u8(r) + 0x30
95-
x = nextx
96-
i--
97-
}
82+
mut mag := u64(n)
9883
if n < 0 {
99-
buf[i] = `-`
100-
i--
84+
b.write_u8(`-`)
85+
// Wrapping unsigned negation yields the correct magnitude even for `min_i64`,
86+
// whose absolute value does not fit in an i64, so this stays allocation-free for
87+
// every input without a special case for the signed 64-bit minimum.
88+
mag = u64(0) - mag
10189
}
102-
unsafe { b.write_ptr(&buf[i + 1], 24 - i) }
90+
b.write_u_decimal(mag)
10391
}
10492

10593
// write_u_decimal appends a decimal representation of the unsigned number `n` into the

vlib/strings/builder.js.v

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,25 +34,16 @@ pub fn (mut b Builder) write_decimal(n i64) {
3434
b.write_u8(0x30)
3535
return
3636
}
37-
if n == min_i64 {
38-
b.write_string(n.str())
39-
return
40-
}
41-
mut buf := [25]u8{}
42-
mut x := if n < 0 { -n } else { n }
43-
mut i := 24
44-
for x != 0 {
45-
buf[i] = u8(x % 10) + 0x30
46-
x = x / 10
47-
i--
48-
}
37+
mut mag := u64(n)
4938
if n < 0 {
50-
buf[i] = `-`
51-
i--
52-
}
53-
for j := i + 1; j <= 24; j++ {
54-
b.write_u8(buf[j])
55-
}
39+
b.write_u8(`-`)
40+
// Wrapping unsigned negation yields the correct magnitude even for `min_i64`,
41+
// whose absolute value does not fit in an i64. It also avoids depending on the
42+
// `min_i64` constant, which the JS backend currently lowers incorrectly, so a
43+
// runtime `min_i64` (e.g. from `'-9223372036854775808'.i64()`) still formats right.
44+
mag = u64(0) - mag
45+
}
46+
b.write_u_decimal(mag)
5647
}
5748

5849
// write_u_decimal appends a decimal representation of the unsigned number `n` into the

vlib/strings/builder_test.v

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ fn test_write_decimal() {
168168
assert sb_i64_str(9223372036854775807) == '9223372036854775807'
169169
assert sb_i64_str(-9223372036854775807) == '-9223372036854775807'
170170
assert sb_i64_str(min_i64) == '-9223372036854775808'
171+
// runtime `min_i64` (parsed, not the constant), whose negation overflows i64:
172+
assert sb_i64_str('-9223372036854775808'.i64()) == '-9223372036854775808'
171173
}
172174

173175
@[manualfree]

0 commit comments

Comments
 (0)