[Webkit-unassigned] [Bug 152028] Unexpected exception assigning to this._property inside arrow function

[bugzilla-daemon at webkit.org]

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

Saam Barati <sbarati at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #267494|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

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

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

r=me

> Source/JavaScriptCore/ChangeLog:8
> +        The issue appeared in case if in arrow function was generated lexical env, and in this case 

grammar: "was generated lexical env" => "created a base-level lexical environment"

> Source/JavaScriptCore/ChangeLog:9
> +        |this| value was loaded from wrong scope. The problem was that loading of the |this| happened too early.

"The problem was that loading of the |this| happened too early" => "The problem was that loading of the |this| happened too early when compiling bytecode because the bytecode generator's scope stack wasn't in sync with the runtime scope stack."

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:525
> +        // Loading of |this| is moving below initializeDefaultParameterValuesAndSetupFunctionScopeStack

I would just remove this entire case from the switch statement. No need for the comment.

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:579
> +    // Loading |this| in arrow function should be done after initializeDefaultParameterValuesAndSetupFunctionScopeStack
> +    // because it contains resolveScope and otherwise it will lead to wrong calculation of address of
> +    // scope in case if lexical env create inside of the arrow function
> +    // because of eval or debug mode

I would write this more simply as:
"Loading |this| inside an arrow function must be done after initializeDefaultParameterValuesAndSetupFunctionScopeStack() because
that function sets up the SymbolTable stack and emitLoadThisFromArrowFunctionLexicalEnvironment() consults the SymbolTable stack"

> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-this-2.js:51
> +var functionConstructorWithEval = function () {
> +  this._id = 'old-value';
> +  this.func = () => {
> +    var f;
> +    eval('10==10');
> +    this._id = 'new-value';
> +    return this._id;
> +  }
> +};

style: 4-space indent.

> Source/JavaScriptCore/tests/stress/arrowfunction-lexical-bind-this-2.js:69
> +function foo() {
> +  let arr = () => {
> +    var x = 123;
> +    function bas() {
> +      return x;
> +    };
> +    this._id = '12345';
> +    return bas();
> +  };
> +  this.arr = arr;
> +};

ditto

> LayoutTests/js/script-tests/arrowfunction-lexical-bind-this.js:93
> +var functionConstructorWithEval = function () {
> +  this._id = 'old-value';
> +  this.func = () => {
> +    var f;
> +    eval('10==10');
> +    this._id = 'new-value';
> +    return this._id;
> +  }
> +};
> +
> +var arrowWithEval = new functionConstructorWithEval();
> +
> +shouldBe("arrowWithEval.func()", '"new-value"');
> +
> +var internal_value_1 = 123;
> +var internal_value_2 = '1234';
> +
> +function foo() {
> +  let arr = () => {
> +    var x = internal_value_1;
> +    function bas() {
> +      return x;
> +    };
> +    this._id = internal_value_2;
> +    return bas();
> +  };
> +  this.arr = arr;

4-space indent.

-- 
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/20151217/85a156ad/attachment.html>