Bug 143012 - ES6: Classes: Early return in sub-class constructor results in returning undefined instead of instance
Summary: ES6: Classes: Early return in sub-class constructor results in returning unde...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-24 12:53 PDT by Joseph Pecoraro
Modified: 2015-03-24 18:19 PDT (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (26.31 KB, patch)
2015-03-24 18:02 PDT, Joseph Pecoraro
rniwa: review+
rniwa: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-03-24 12:53:52 PDT
* 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`.
Comment 1 Joseph Pecoraro 2015-03-24 13:13:42 PDT
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.
Comment 2 Joseph Pecoraro 2015-03-24 14:13:15 PDT
I think I see the issue. I'll investigate.
Comment 3 Joseph Pecoraro 2015-03-24 18:02:58 PDT
Created attachment 249370 [details]
[PATCH] Proposed Fix
Comment 4 Ryosuke Niwa 2015-03-24 18:10:46 PDT
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.
Comment 5 Joseph Pecoraro 2015-03-24 18:19:28 PDT
http://trac.webkit.org/changeset/181924