Skip to content

fix: Fix wrong string comparision when sizes are differ #1583

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Dec 16, 2020
Merged

fix: Fix wrong string comparision when sizes are differ #1583

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7adde77
fix
MaxGraey Dec 14, 2020
b98654f
more tests
MaxGraey Dec 14, 2020
79f808e
do the same for string sorting
MaxGraey Dec 15, 2020
f080098
more tests
MaxGraey Dec 15, 2020
ed2e5df
move tests to other place
MaxGraey Dec 15, 2020
b40dd86
hide ts lang warnings
MaxGraey Dec 15, 2020
cfcdf20
update
MaxGraey Dec 15, 2020
9fe4b24
Update std/assembly/util/sort.ts
MaxGraey Dec 15, 2020
e381380
update fixtures
MaxGraey Dec 15, 2020
0b1e2f1
refactor
MaxGraey Dec 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions std/assembly/string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ import { Array } from "./array";
var rightLength = right.length;
if (!rightLength) return true;
// @ts-ignore: string <-> String
return compareImpl(left, 0, right, 0, min(leftLength, rightLength)) > 0;
var res = compareImpl(left, 0, right, 0, min(leftLength, rightLength));
return res == 0 ? leftLength > rightLength : res > 0;
}

@operator(">=") private static __gte(left: String, right: String): bool {
Expand All @@ -136,7 +137,8 @@ import { Array } from "./array";
var leftLength = left.length;
if (!leftLength) return true;
// @ts-ignore: string <-> String
return compareImpl(left, 0, right, 0, min(leftLength, rightLength)) < 0;
var res = compareImpl(left, 0, right, 0, min(leftLength, rightLength));
return res == 0 ? leftLength < rightLength : res < 0;
}

@operator("<=") private static __lte(left: String, right: String): bool {
Expand Down
5 changes: 3 additions & 2 deletions std/assembly/util/sort.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ export function COMPARATOR<T>(): (a: T, b: T) => i32 {
if (a === b || a === null || b === null) return 0;
var alen = changetype<string>(a).length;
var blen = changetype<string>(b).length;
if (!alen && !blen) return 0;
if ((alen | blen) == 0) return 0;
if (!alen) return -1;
if (!blen) return 1;
return compareImpl(changetype<string>(a), 0, changetype<string>(b), 0, <usize>min(alen, blen));
let res = compareImpl(changetype<string>(a), 0, changetype<string>(b), 0, <usize>min(alen, blen));
return res == 0 ? alen - blen : res;
};
} else {
return (a: T, b: T): i32 => (i32(a > b) - i32(a < b));
Expand Down
13 changes: 10 additions & 3 deletions tests/compiler/std/array.optimized.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6315,6 +6315,7 @@
(func $~lib/util/sort/COMPARATOR<~lib/string/String|null>~anonymous|0 (param $0 i32) (param $1 i32) (result i32)
(local $2 i32)
(local $3 i32)
(local $4 i32)
local.get $0
call $~lib/rt/pure/__retain
local.set $0
Expand All @@ -6333,23 +6334,22 @@
select
select
br_if $folding-inner0
i32.const 0
local.get $1
i32.const 20
i32.sub
i32.load offset=16
i32.const 1
i32.shr_u
local.tee $2
i32.eqz
local.get $0
i32.const 20
i32.sub
i32.load offset=16
i32.const 1
i32.shr_u
local.tee $3
select
i32.or
i32.eqz
br_if $folding-inner0
local.get $3
i32.eqz
Expand Down Expand Up @@ -6380,10 +6380,17 @@
i32.gt_s
select
call $~lib/util/string/compareImpl
local.set $4
local.get $0
call $~lib/rt/pure/__release
local.get $1
call $~lib/rt/pure/__release
local.get $4
local.get $3
local.get $2
i32.sub
local.get $4
select
return
end
local.get $0
Expand Down
2 changes: 1 addition & 1 deletion tests/compiler/std/array.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1067,4 +1067,4 @@ export class ArrayU32 extends Array<u32> {}
export class ArrayU8 extends Array<u8> {}
export class ArrayStr extends Array<string> {}
// TODO:
// export class ArrayArrayI32 extends Array<Array<i32>> {}
// export class ArrayArrayI32 extends Array<Array<i32>> {}
72 changes: 48 additions & 24 deletions tests/compiler/std/array.untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11778,13 +11778,10 @@
call $~lib/string/String#get:length
local.set $4
local.get $3
i32.eqz
if (result i32)
local.get $4
i32.eqz
else
i32.const 0
end
local.get $4
i32.or
i32.const 0
i32.eq
if
i32.const 0
local.set $2
Expand Down Expand Up @@ -11833,11 +11830,22 @@
select
call $~lib/util/string/compareImpl
local.set $2
local.get $2
i32.const 0
i32.eq
if (result i32)
local.get $3
local.get $4
i32.sub
else
local.get $2
end
local.set $5
local.get $0
call $~lib/rt/pure/__release
local.get $1
call $~lib/rt/pure/__release
local.get $2
local.get $5
)
(func $std/array/assertSorted<~lib/string/String|null>@varargs (param $0 i32) (param $1 i32)
(local $2 i32)
Expand Down Expand Up @@ -12830,13 +12838,10 @@
call $~lib/string/String#get:length
local.set $4
local.get $3
i32.eqz
if (result i32)
local.get $4
i32.eqz
else
i32.const 0
end
local.get $4
i32.or
i32.const 0
i32.eq
if
i32.const 0
local.set $2
Expand Down Expand Up @@ -12885,11 +12890,22 @@
select
call $~lib/util/string/compareImpl
local.set $2
local.get $2
i32.const 0
i32.eq
if (result i32)
local.get $3
local.get $4
i32.sub
else
local.get $2
end
local.set $5
local.get $0
call $~lib/rt/pure/__release
local.get $1
call $~lib/rt/pure/__release
local.get $2
local.get $5
)
(func $std/array/assertSorted<~lib/string/String>@varargs (param $0 i32) (param $1 i32)
(local $2 i32)
Expand Down Expand Up @@ -32588,13 +32604,10 @@
call $~lib/string/String#get:length
local.set $4
local.get $3
i32.eqz
if (result i32)
local.get $4
i32.eqz
else
i32.const 0
end
local.get $4
i32.or
i32.const 0
i32.eq
if
i32.const 0
local.set $2
Expand Down Expand Up @@ -32643,11 +32656,22 @@
select
call $~lib/util/string/compareImpl
local.set $2
local.get $2
i32.const 0
i32.eq
if (result i32)
local.get $3
local.get $4
i32.sub
else
local.get $2
end
local.set $5
local.get $0
call $~lib/rt/pure/__release
local.get $1
call $~lib/rt/pure/__release
local.get $2
local.get $5
)
(func $~lib/array/Array<~lib/string/String>#sort@varargs (param $0 i32) (param $1 i32) (result i32)
(local $2 i32)
Expand Down
2 changes: 1 addition & 1 deletion tests/compiler/std/string-encoding.optimized.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2591,7 +2591,7 @@
if
i32.const 0
i32.const 1504
i32.const 740
i32.const 742
i32.const 7
call $~lib/builtins/abort
unreachable
Expand Down
2 changes: 1 addition & 1 deletion tests/compiler/std/string-encoding.untouched.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4388,7 +4388,7 @@
if
i32.const 0
i32.const 480
i32.const 740
i32.const 742
i32.const 7
call $~lib/builtins/abort
unreachable
Expand Down
Loading