search

HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.2k stars 658 forks source link

[std][hl] Fix array pos check, force UInt #11810

Open yuxiaomao opened 3 weeks ago

yuxiaomao commented 3 weeks ago

Closes https://github.com/HaxeFoundation/hashlink/issues/723

Seems to be some missing cast from https://github.com/HaxeFoundation/haxe/pull/8408

Simn commented 3 weeks ago

This entire UInt business here looks pretty wonky, can't we just do a pos < 0 || pos >= length check like normal humans?

yuxiaomao commented 3 weeks ago

I searched UInt in std and got for example

https://github.com/HaxeFoundation/haxe/blob/fe0d3e34abb5de658d160eba0b8cc126fab77edc/std/hl/_std/String.hx#L50-L55

So I thought it's the "normal" usage. Can change to pos < 0 || pos >= length too if you prefer.

UInt seems to save a comparison (using OJULt)

Simn commented 3 weeks ago

Doesn't this just work incidentally because something like -1 becomes a large number that then exceeds length? I don't see how this is sound because with a sufficiently small number you could end up with something that's < length after casting.

yuxiaomao commented 3 weeks ago

Negative Int has the most significant bit at 1, with INT_MIN it's 0x80000000 and is always bigger (unsigned operation) than a normal length ranged from 0 to INT_MAX (0x7FFFFFFF)

Simn commented 3 weeks ago

Ah right, because the length is Int, not UInt... Well, if this is how you want to handle this then I'm fine with it.