HaxeFoundation / haxe

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

unconsistent default value across targets in trace. #3877

Open sebpatu opened 9 years ago

sebpatu commented 9 years ago

in that test : http://try.haxe.org/#8Bf77 you can see different behaviors depending on platforms, and basic type, for the default value that is traced.

good thing is that the nullability test is ok, but the traced value is not showing 'null'. on neko i get expected result: (except for current bug for UInt): every trace shows 'null' value, and every test '== null' is true.

for js target of try haxe: if variables are not nullable, i get undefined value, except for Bool type. i think its a bit to much Platform specific, and not enough haxe consistent behavior. And its quite confusing for coder to have a value different from 'null' in trace, and a test '==null' that is true.

i would expect same result for not Null<> variable and for Null<> ones. what do you think?

Simn commented 9 years ago

This is very confusing, can you just make an example of the stuff you consider broken?

sebpatu commented 9 years ago

That was the purpose of the try haxe link. I don't think its broken, i may have been always like that, but i think its a bit inconsistent in haxe context.

But to make it short: consider those 2 variables (no default value specify): public var intTest:Int; public var boolTest:Bool;

test the output of those 2 lines: trace("Int Value= "+intTest+" Is null="+(intTest == null)); trace("Bool Value= "+boolTest+" Is null="+(boolTest == null));

on neko target i get what i think is what should always be the result for any other target: Int Value= null Is null=true Bool Value= null Is null=true

on js target i get: Int Value= undefined Is null=true Bool Value= null Is null=true

its inconsistent between js target and neko target its inconsistent between Int and Bool types for the same target js. its not haxe syntax consistent to have different results between: trace of intTest value being 'undefined', and the result of test (intTest == null) being true, suggesting that intTest value is null! thats confusion between 'null' (haxe context) and 'undefined' (javascript context)

conclusion: the trace result is broken.

nadako commented 9 years ago

that's more of a string concatentation issue. Int and Float are concatenated as is, while Bool is passed through Std.string which does check for null, hence the different result (see https://github.com/HaxeFoundation/haxe/blob/development/typer.ml#L2007)

sebpatu commented 9 years ago

Well done nadako, So wouldn't it be more consistent to use Std.string for all outputs?

nadako commented 9 years ago

well that's an optimization i believe, I don't know how should it be. probably the same as with Bool. i'll leave this to @ncannasse

ncannasse commented 9 years ago

@Simn we should do something for Null (check == null before calling toString)

Regarding undefined VS null, I don't think we should do anything here

Simn commented 9 years ago

There's some discussion in https://github.com/HaxeFoundation/haxe/issues/3376, though that's mostly me discussing with myself.

ncannasse commented 9 years ago

I guess that's a bit complementary case here. We could fix the null + toString first and see what we do with eq operator at another time

sebpatu commented 9 years ago

@ncannasse Regarding undefined VS null why we should not do anything ?

Simn commented 9 years ago

Someone please post a concise example of the toString problem.

Simn commented 9 years ago

Or is this the same as #3875 minus the undefined part?

nadako commented 9 years ago
class Main {
    static var b:Bool;
    static var i:Int;

    static function main() {
        var v = "" + i;
        var v2 = "" + b;
        trace(v);
        trace(v2);
    }
}

generates

Main.main = function() {
    var v = "" + Main.i;
    var v2 = "" + Std.string(Main.b);
    console.log(v);
    console.log(v2);
};

thus outputs

undefined
null
ncannasse commented 9 years ago

@nadako that's the one I propose we don't fix, because that would require to check everything for null before converting to String on non static platforms

@Simn My actual problem is the following : Std.string( (null : Null<UInt>) ) == "NaN". That's because we call UInt.toString whereas we should have (v == null ? "null" : UInt.toString(v)) -- we can't except each toString method to handle the "null" case, since "this" is usually not nullable.

ncannasse commented 9 years ago

Just to clarify : Std.string(null) should always be "null", whatever the type of "null" is :)

Simn commented 9 years ago

Let's do this after 3.2. I'm concerned about complex expressions which may require creating a block with a temp var.

Simn commented 8 years ago

type_binop2 has this:

let to_string e =
    let rec loop t = match classify t with
        | KAbstract ({a_impl = Some c},_) when PMap.mem "toString" c.cl_statics ->
            call_to_string ctx c e
        | KInt | KFloat | KString -> e
        | _ -> (* make Std.string call *)

That code is quite ancient. Do we really want to change this?