Bug 143012 – ES6: Classes: Early return in sub-class constructor results in returning undefined instead of instance

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Joseph Pecoraro]

* SUMMARY
Early return in sub-class constructor results in returning undefined instead of instance

* TEST
<script>
class Base { constructor(){ return; } }
class Derived extends Base {
    constructor()
    {
        super();

        // This early return breaks things.
        return;
    }
}

var x = new Derived;
console.log(x); // undefined, should be new Derived.
</script>


* EXPECTED
Derived instance in `x`.

* ACTUAL
undefined in `x`.

[Joseph Pecoraro]

I think the relevant portion is [[Construct]]:
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-ecmascript-function-objects-construct-argumentslist-newtarget

Which has:

>  13. If result.[[type]] is return, then
>      a. If Type(result.[[value]]) is Object, return NormalCompletion(result.[[value]]).
>      b. If kind is "base", return NormalCompletion(thisArgument).
>      c. If result.[[value]] is not undefined, throw a TypeError exception.
>  14. Else, ReturnIfAbrupt(result).
>  15. Return envRec.GetThisBinding().


That seems weird to me. It sounds like if you early return from a base class constructor, you will get the "thisArgument". And if you early return from a derived class constructor we return "envRec.GetThisBinding()".

Either way, my interpretation is that if we do an implicit return of undefined, we would get some "this" object.

[Joseph Pecoraro]

I think I see the issue. I'll investigate.

[Joseph Pecoraro]

Created attachment 249370 [details]
[PATCH] Proposed Fix

[Ryosuke Niwa]

Comment on attachment 249370 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=249370&action=review

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1943
> +        if (!derived)
> +            emitUnaryNoDstOp(op_ret, &m_thisRegister);
> +        else {

I think this reads better if we checked if (derived) and put emitUnaryNoDstOp(op_ret, &m_thisRegister); after the if block.

[Joseph Pecoraro]

http://trac.webkit.org/changeset/181924