[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

Saam Barati <sbarati at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #262397|review?                     |review-
              Flags|                            |

--- Comment #8 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 262397
  --> https://bugs.webkit.org/attachment.cgi?id=262397
Patch

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:485
> +            RefPtr<RegisterID> parentScope = m_lexicalEnvironmentRegister

We have a byte code variable m_topMostScope that does what you're doing here, but I think this logic is wrong.
Consider this program:
constructor() {
    if (c) {
         let x = 20;
         function captureX() { }
         if (c) {
            let x = 20;
            function captureX() { return x; }
            let arr = (blah) => blah;
         }
    }
}

The "arr" won't be created with the parent scope that contains the "this".

I think you just want a resolveScope followed by a getFromScope.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2949
> +        emitPutToScope(scopeRegister(), thisVar, thisRegister(), ThrowIfNotFound, NotInitialization);

I think special casing "this" as a new thing both in terms of a resolve type
and in terms of a variable on JSScope is the wrong way to go about implementing this feature.

Here is one suggestion on how to solve this differently:
Anytime a function has an arrow function nested inside of it,
the parent function should create a lexical environment. Once this parent
function also creates the "this" variable, it should place it inside
the lexical environment it created. (This solves the problem in this code which keeps putting
the "this" into the activation every time an arrow function is created
even if "this" hasn't changed). Any time you make a call to super()
and you have a nested arrow function, you update the "this" inside
the lexical environment. Child functions that read from "this" can
just do so the normal way: resolveScope() then getFromScope().

The parent function that has the "this" inside the lexical environment
should just do what it normally does for lexical environments. The "this"
identifier should have a slot inside the symbol table, etc. I think this
would take away almost all this special case code for "this". Then, the "thisNode",
when inside an arrow function, should be smart and load the "this" from
the lexical environment using resolveScope() then getFromScope(). I believe
this suggested solution will cause "this" inside an environment to just work
for the most part.

> Source/JavaScriptCore/jit/JITOperations.cpp:1995
> +    if (getPutInfo.resolveType() == LexicallyBoundVar) {

I think special casing this is wrong. We should just be able to put the "this" identifier into an environment and have this code work.

-- 
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/20151004/f5376aed/attachment-0001.html>