HaxeFoundation / haxe

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

Ambiguous specification of haxe.io.Output.writeByte() #4888

Open kaikoga opened 8 years ago

kaikoga commented 8 years ago
class Main {
    public static function main():Void {
        Sys.stdout().writeByte(-159);
    }
}

Above code behaves inconsistent across targets. neko throws error, cpp and java exits with no output, php outputs an a, ...

Actually, haxe.io.Output.writeByte() won't accept anything outside 0...256 range but is not documented. At least we need an "If x is outside of the unsigned byte range, the result is unspecified" clause.

Or, as a more robust fix, maybe we need a writeUInt8(x:Int):Void which throws haxe.io.Error.Overflow if outside range, to complement other methods.

Submitted this as an Issue because I'm not sure which PR should I make.

Simn commented 8 years ago

I don't know, do we really have to mention everywhere what a Byte is? We can do it, but then we should find all cases where this applies.

delahee commented 8 years ago

2c, in doubt, refer to the best doc out there ( mine is c#/directx ). ( https://msdn.microsoft.com/fr-fr/library/system.io.filestream.writebyte(v=vs.110).aspx ) I quite like that they have an abstract Byte that refers to what is a byte.

Since putting an abstract here would be quite tedious, I think hinting at what is a byte is fair. At least hinting definition domain of parameters is fair practice. I also think it's fair since writeDouble hints for 64-bit floating point itself.

Throwing an exception + bound checking is meh...

ncannasse commented 8 years ago

I agree we need at least documentation, or maybe ensuring cross platform behavior by adding &0xFF where it's missing wouldn't be a bad thing either.

kaikoga commented 8 years ago

FYI, the actual mistake I made was thinking that a signed byte parameter would be accepted and writing output.writeByte(boolValue ? -1 : 0);, and overlooking it because I was testing mainly in swf target, where ByteArray.writeByte(-1) ignores the high 24 bits and take the LSB. The AS3 doc even states so... http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/utils/ByteArray.html#writeByte%28%29

Of course I agree Bytes is not ByteArray.

Simn commented 8 years ago

Does anyone feel like sending a documentation pull request for this?

ncannasse commented 8 years ago

@Simn could we not instead enforce cross plaform behavior? Adding a few &0xFF here and there shouldn't be a big issue.

Simn commented 8 years ago

I'm not sure about that... shouldn't byte operations be as fast as possible? Also, there's a good chance that it's not gonna be correct anyway if you pass values outside of byte range into it.

ncannasse commented 8 years ago

@Simn haxe.io.Output is already an abstract implementation meant more for its genericity than for its raw performances so adding & 0xFF is not a big difference here. Some platforms already perform this operation so having a cross platform behavior is better in that case.