jsx / JSX

JSX - a faster, safer, easier JavaScript
http://jsx.github.io/
MIT License
1.46k stars 102 forks source link

disable side effect in default parameter. #327

Open tuchida opened 10 years ago

tuchida commented 10 years ago

I run this JSX's code.

class _Main {

    static function fn(a : number, b : number = ++a) : void {
        log a;
    }

    static function main(args : string[]) : void {
        _Main.fn(1);
    }
}

I expected to output 2, but 1. (Sorry. I missed https://github.com/jsx/JSX/pull/326)

This is compiled JavaScript code.

function _Main$fn$N(a) {
    return _Main$fn$NN(a, ++a);
};

I think it should be this.

function _Main$fn$N(a) {
    var $arg2 = ++a;
    return _Main$fn$NN(a, $arg2);
};
tuchida commented 10 years ago

I was trying to fix https://github.com/tuchida/JSX/commit/8770306bddedd1d391b6a2ae15d67f7fd15eba45 . I found two problems.

First. This code fails to compile when add --optimize release --minify options. I reported https://github.com/jsx/JSX/issues/328 .

static function f1(a : number, b : number = ++a) : void {
    log a;
    log b;
}

Second, It is not expected in the case of closure has side effects. I do not know how to deal with this case.

static function f2(a : number, b : () => number = () => ++a) : void {
    log b();   // expect 2
    log a;     // expect 2 but 1
}

Do you have a good idea?

kazuho commented 10 years ago

Thank you for looking into the issue.

IMO we should not allow the modification of preceding arguments within the default argument expression. Or please let me know if you have any reason to support such behaviour.

tuchida commented 10 years ago

For example.

class _Main {

    static function fn(a : number, b : () => number = () => a) : void {
        log b();  // expect 1
        a = 10;
        log b();  // expect 10 but 1
    }

    static function main(args : string[]) : void {
        _Main.fn(1);
    }
}

If not allow the modification of a, is there compatibility problem?

tuchida commented 10 years ago

It might work if the output JavaScript code like this.

class _Main {

    static function fn(a : number, b : () => number = () => a) : void {
        log b();  // expect 1
        a = 10;
        log b();  // expect 10 but 1
    }

    static function main(args : string[]) : void {
        _Main.fn(1);
        _Main.fn(10, () => 20);
    }
}
var NO_PARAM = {};

function _Main$fn$N(a, b) {
    if (b === NO_PARAM) {
        b = function () {
            return a;
        };
    }
    console.log(b());
    a = 10;
    console.log(b());
}

function _Main$main(args) {
    _Main$fn$N(1, NO_PARAM);
    _Main$fn$N(10, function() {
      return 20;
    });
};

Can decreased number of function calls. But many changes you will probably need.

kazuho commented 10 years ago

If not allow the modification of a, is there compatibility problem?

The answer is no, practically, since the feature has only been introduced lately.

It might work if the output JavaScript code like this. (snip) Can decreased number of function calls. But many changes you will probably need.

Such design was considered when we introduced the default arguments, but had been dismissed for two reasons: that the model is complicated, and that it would be slow. Current design will run fast together with the inline optimizer, since the default argument expressions can be inline-expanded in the caller side by the optimizer.

tuchida commented 10 years ago

Thanks!

IMO we should not allow the modification of preceding arguments within the default argument expression.

How do you like https://github.com/tuchida/JSX/commit/bbaabac8df01b921d0dbc6e6c972d2e92df8d6ac ?

kazuho commented 10 years ago

Thank you for looking into the issue.

What I mean is that modification of preceding arguments should disallowed only within the default argument expressions, and it should continue to be allowed at other locations. i.e.

function fn(a : number, b : number = 10) : void {
    a = 11;
}

should be considered valid, OTOH

function fn(a : number, b : number = ++a) : void {
}

should be invalid.

tuchida commented 10 years ago

This case.

    static function fn(a : number, b : () => number = () => a) : void {
        log b();  // expect 1
        a = 10;
        log b();  // expect 10 but 1
    }

    static function main(args : string[]) : void {
        _Main.fn(1);
    }

a is changed outside the default argument expressions. Should b() always returns 1 even after the change?

Or should disallowed modification of 'a', because it is referenced in the default argument expressions?

kazuho commented 10 years ago

Thank you for the clarification.

a is changed outside the default argument expressions. Should b() always returns 1 even after the change?

IMO b() should return 1 even after a is being changed, since it is more consistent to consider the default argument expressions as something that is expanded at the caller-side rather than the opposite.

Below is a valid example using default argument expression that prints Derived:123. Such behaviour would seem natural if the expression is considered to be expanded at the caller side (since the intended call signature would be Base#f(:number)), but would seem unnatural if it were to be considered as expanded at the callee-side (i.e. method with no arguments is only defined in the base class, and thus the output should be Base:123 in such case).

class Base {
    function f(n : number = 123) : void {
        log "Base:" + n;
    }
}

class Derived extends Base {
    override function f(n : number) : void {
        log "Derived:" + n;
    }
}

class _Main {
    static function main(args : string[]) : void {
        (new Derived).f();                            // output: Derived:123
    }
}
kazuho commented 10 years ago

Having said that, it might not be necessary to disallow the modification of preceding arguments within default argument expressions. Just documenting the behavior might be sufficient.

tuchida commented 10 years ago

The very interesting case of override method. I do not know which is the best.

kazuho commented 10 years ago

I do not know which is the best.

The override example illustrates the feature of the compiler that is already being used. That is why I believe it is better to consider the default argument expressions as something that is expanded at the caller-side which in turn means that the return value of b() should not be affected by the modification of a, unless there is a strong reason not to do so.