[Webkit-unassigned] [Bug 149338] [ES6] Arrow function created before super() causes TDZ, should it?

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=149338

GSkachkov <gskachkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #264078|1                           |0
        is obsolete|                            |

--- Comment #53 from GSkachkov <gskachkov at gmail.com> ---
Comment on attachment 264078
  --> https://bugs.webkit.org/attachment.cgi?id=264078
Patch

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

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:849
>> +    m_arrowFunctionContextLexicalEnvironmentRegister = emitMove(newBlockScopeVariable(), m_scopeRegister);
> 
> I was wrong about advising you on this earlier.
> I forgot that pushLexicalScopeInternal already creates a variable for us.
> Because you have captured variables, we will create a block scoped variable for
> the scope. 
> So all you have to do is this:
> m_arrowFunctionContextLexcialEnvironmentRegister = m_symbolTableStack.last().m_scope.
> Feel free to add asserts around the pushLexicalScopeInternal call to make sure it actually adds exactly one item to the symbol table stack.

Done

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2989
>> +        if (m_codeType == FunctionCode)  {
> 
> I realized we don't need this.
> We should only need to do this once per function.
> If you look how new_target is materialized now, it happens
> before we create a "this". So I think it'd be wrong to update
> this value over time. It should just happen once at the very beginning of the program.

I've split this into two methods

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:3881
>> +{
> 
> There are a few interesting things to consider here that seem like they are problems.
> 1) consider the problem of using the literal string "__proto__":
> class A extends B {
>     let arr1 = () => {
>         let __proto__ = "blah";
>         let arr2 = () => super();
>         arr2();
>     }
> }
> This strategy worked with "this" because no variable can be named "this". But variables can be named "__proto__"
> 
> 2) what happens when __proto__ is overwritten (I'm not really sure if this is a problem or what's supposed to happen with respect to the specification, but it's probably worth looking into).
> class A extends B {
>     let arr1 = () => { ... }
>     A.__proto__ = null;
>     arr1();
> }
> I wonder what happens just in normal classes with this.
> 
> 3) If we're storing values in m_calleeRegister, maybe we're doing this wrong. I think this might break stack traces because it will look like an arrow functions parent frame is displayed twice on a stack trace if an error is thrown.
> It's worth considering this problem.
> 
> For all problems above, if they really are problems, we should have tests for them, too.

Added additional tests to arrowfunction-lexical-bind-supercall-2.js module for cases that you mention.
Stack trace looks correct
for instance for following code:
var errorStack;

var ParentClass = class ParentClass {
  constructor() {
    try {
      this.idValue = 'testValue';
      throw new Error('Error');
    } catch (e) {
      errorStack  = e.stack;
    }
  }
};

var ChildClass = class ChildClass extends ParentClass {
  constructor () {
    var arrowInChildConstructor = () => {
      var nestedArrow = () => {
        super();
      }
      nestedArrow();
    };
    arrowInChildConstructor();
  }
};


Stack trace is:
ParentClass at test_class_14.js:9:22
nestedArrow at test_class_14.js:20:14
arrowInChildConstructor at test_class_14.js:23:18
ChildClass at test_class_14.js:26:28
global code at test_class_14.js:31:25

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20151102/495dfe35/attachment.html>