<rdar://problem/19467094>

Created attachment 244682 [details]
Super naive WIP 1

With this patch, the following code runs successfully.  I still need to add the support for super() as well as make the object initialization check (throw ReferenceError whenever this is accessed before the initialization) work.

class A {
    someMethod() { alert('hi'); }
    static someStaticMethod(o) { o.someMethod(); }
    constructor() {}
    static constructor() {}
};
class B extends A {
    constructor() { }
}
var b = new B();
A.someStaticMethod(b);

Comment on attachment 244682 [details]
Super naive WIP 1

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

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:2771
> +        // FIXME: Throw TypeError if isConstructor(parentClass) is false or it's a generator function.

I'm honestly leaning towards planting a private property on the type and using that to make you decision (e.g. foo.@IsConstructor or some such)

> > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:2771
> > +        // FIXME: Throw TypeError if isConstructor(parentClass) is false or it's a generator function.
> 
> I'm honestly leaning towards planting a private property on the type and
> using that to make you decision (e.g. foo.@IsConstructor or some such)

Another option is to use a special JSFunction subclass for constructors.

Created attachment 244853 [details]
Added "super" support

The new WIP is still missing two major pieces:

1. Throw ReferneceError when "this" is still in "uninitialized" state (i.e. before "super()" is called) in a derived class' constructor.
2. Auto generate the constructor when it's not supplied.

I think I'll do 2 in a separate follow-up patch since JSC doesn't have a facility to generate a Miranda function.

I've talked about 1 with Oliver and Gavin, and the approach we came up so far (if I remember correctly) is to allocate a local variable/register that stores "uninitialized-ness" of the "this" variable. Inside ThisNode or wherever else "this" is accessed, we'll generate a code to check this state and throw ReferenceError when it's not initialized.

Once that's implemented, we can optimize this in the parser as well as in the bytecode generator for common cases.  e.g. in cases where we can guarantee that super() is definitively called before "this" is used, we can omit this check.  Even if we didn't implement such an optimization, DFG might be able to optimize them away.

> I've talked about 1 with Oliver and Gavin, and the approach we came up so
> far (if I remember correctly) is to allocate a local variable/register that
> stores "uninitialized-ness" of the "this" variable.

I think you can just use undefined to mean uninitialized.

The ThisNode in the AST can emit explicit branches around access to 'this' to check for undefined, if and only if you're compiling a constructor that extends a builtin.

The DFG will probably dead-code-eliminate those branches in simple cases. The FTL should get them in all cases.

Created attachment 244964 [details]
Added ReferenceError for this inside derived class' constructor

I've added the support for throwing ReferenceError inside a derived class' constructor.  There is one ugliness here, which is having to remember whether a given constructor is for a derived class or not.  I don't think we can deduce this from prototype or constructor as far as I read the spec so I'm introducing a yet another private property name to store this information on the constructor object.

Now I need to figure out how to pass an argument to eval operator so that every access to this inside an eval operator in a derived class' constructor would check the extra register allocated in BytecodeGenerator for the constructor.

Created attachment 245050 [details]
Set this to undefined instead of throwing

Geoff, Gavin, and I had an extensive discussion on this matter and decided that we should just set "this" to undefined for now. I've emailed es-discuss to change the spec accordingly.

(In reply to comment #9)
> Created attachment 245050 [details]
> Set this to undefined instead of throwing
> 
> Geoff, Gavin, and I had an extensive discussion on this matter and decided
> that we should just set "this" to undefined for now. I've emailed es-discuss
> to change the spec accordingly.

I take it this is no longer true? ;-)

Created attachment 245268 [details]
Updated for ToT

Created attachment 245506 [details]
Diff against my patch on 140918

I've also made the [[Construct]] use the right structure. This involves changing how op_create_this works. At the moment, we use the "callee" object to find the structure for the new object but we need to be using the most derived class's constructor (called newTarget).

After discussing with Gavin & Geoff, I'm using Gavin's idea to use "this" register for this purpose. The idea is to put the newTarget into the "this" register and then op_create_this will obtain the structure out of that value instead of the callee object.

The alternatives invoke modifying the way arguments work in "construct" and that seemed much worse hack than this one.

Created attachment 247489 [details]
Added op_check_tdz in 64-bit LLint & Baseline JIT

I think I'm going to just implement subclassing without TDZ first, and then add TDZ in a separate patch to keep the patch sizes small.

Attachment 247489 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/parser/Nodes.h:1622:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/parser/Nodes.h:1623:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:83:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:175:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:543:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
Total errors found: 5 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

This has been fixed for a while.