HaxeFoundation / haxe

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

DynamicAccess should return the value it is assigned #3517

Closed ghalle closed 9 years ago

ghalle commented 9 years ago

Chain assignment like this won't work with the error: "Arguments and variables of type Void are not allowed"

var cache:DynamicAccess<Int> = { };
var foo = cache['bar'] = 0;

That is because the set function return void:

@:arrayAccess
    public inline function set(key:String, value:T):Void Reflect.setField(this, key, value);

Ideally I would return the setField call but setField return Void and not the value set. I tried this workaround:

@:arrayAccess
    public inline function set(key:String, value:T):T {
        Reflect.setField(this, key, value);
        return value;
    }

But it is not ideal as it create 2 separate statement and with complex assignment this creates multiple variables and statements.

nadako commented 9 years ago

maybe we could make Reflect.setField return a value...

nadako commented 9 years ago

I took a quick look at it and it seems that:

Simn commented 9 years ago

I would prefer if we didn't change Reflect.setField. The main purpose of setters returning values is to chain them, which is not something you typically do when using Reflect. It's better to implement this on the syntax side of things, i.e. DynamicAccess.set.

ncannasse commented 9 years ago

Agree with @Simn. And let the code optimizer get rid of the unused statement as it should

nadako commented 9 years ago

I changed DynamicAccess.set to return given value and added an optimized version for JavaScript that doesn't generate extra vars and closures. That looks good enough for now.