`this` in top level Arrow Function in Module Context should be `undefined`
Reported by
azuc...@gmail.com,
Dec 3 2017
|
|||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Firefox/52.0
Steps to reproduce the problem:
1. Run following code in module context
<script type="module">
const strictTopLevelArrowFunctionThis = () => {
return this;
};
console.log(trictTopLevelArrowFunctionThis()); // `Window` in Chrome
</script>
I've created comparison table of `this` value.
- https://azu.github.io/what-is-this/
What is the expected behavior?
The `this` should be `undefined`.
My understand that the `this` in Top level Arrow Function in Module Context refer to `[[ThisValue]]` of Module Environment Records.
Module Environment Records's `GetThisBinding ( )` return `undefined`.
- https://tc39.github.io/ecma262/#sec-module-environment-records
- https://tc39.github.io/ecma262/#sec-module-environment-records-getthisbinding
What went wrong?
Chrome output `Window` object, but other browsers output `undefined`.
Did this work before? N/A
Chrome version: 64.0.3282.5 (Official Build) canary Channel: n/a
OS Version: OS X 10.12
Flash Version: Shockwave Flash 27.0 r0
MSEdge 16.16299: `undefined`
Firefox 58.0b8(`dom.moduleScripts.enabled` flag): `undefiend`
macOS Safari 11.0.1 (12604.3.5.1.1): `undefiend`
,
Dec 4 2017
,
Dec 4 2017
Yes, it should be undefined. I'm not sure what's going on here, it seems very bizarre:
const fn1 = () => {return this};
const fn2 = () => {return this};
console.log(fn1());
console.log(fn2());
This prints Window twice.
const fn1 = () => this;
const fn2 = () => this;
console.log(fn1());
console.log(fn2());
This prints undefined twice.
const fn1 = () => this;
const fn2 = () => {return this};
console.log(fn1());
console.log(fn2());
This ALSO prints undefined twice.
I will investigate further but I may not have the time until Wed/Thu. Adam, feel free to have a look in the meantime if you have a chance.
,
Dec 4 2017
This seems related to lazy parsing.
$ cat ../this.js
const foo = () => {return this};
print(foo());
$ d8 Debug --no-lazy --module ../this.js
undefined
$ d8 Debug --module ../this.js
[object global]
,
Dec 5 2017
Looked more closely today with adamk and we believe we know how to fix it. Planning to have a CL ready this week.
,
Dec 5 2017
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/c3bd741efd29b8c6b322cea62f045e971cd7d183 commit c3bd741efd29b8c6b322cea62f045e971cd7d183 Author: Georg Neis <neis@chromium.org> Date: Tue Dec 12 12:09:49 2017 Fix "this" value in lazily-parsed module functions. When preparsing top-level functions in a module, we didn't track unresolved variables. Consequently, "this" ended up referencing the global "this", which has the wrong value (in a module "this" is supposed to be the undefined value). This patch fixes that. This also lets us stop forcing context allocation of all variables in module scopes, which the patch takes care of as well. Bug: chromium:791334 Change-Id: Ifac1f1adc033f3facfb3d29dd4bca32ee27bffcf Reviewed-on: https://chromium-review.googlesource.com/808938 Reviewed-by: Marja Hölttä <marja@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#50025} [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/src/ast/scopes.cc [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/src/ast/scopes.h [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/src/parsing/parser.cc [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/cctest/interpreter/bytecode_expectations/Modules.golden [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/cctest/test-parsing.cc [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/debugger/debug/debug-modules-set-variable-value.js [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/debugger/debug/harmony/modules-debug-scopes2.js [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt [modify] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/inspector/debugger/evaluate-on-call-frame-in-module.js [add] https://crrev.com/c3bd741efd29b8c6b322cea62f045e971cd7d183/test/mjsunit/regress/regress-791334.js
,
Dec 12 2017
,
Dec 12 2017
Thanks for fixint it!
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/62f09de9ab9a743cb929b327818b86d027cc888b commit 62f09de9ab9a743cb929b327818b86d027cc888b Author: Michael Achenbach <machenbach@chromium.org> Date: Tue Dec 12 14:08:25 2017 Revert "Fix "this" value in lazily-parsed module functions." This reverts commit c3bd741efd29b8c6b322cea62f045e971cd7d183. Reason for revert: Breaks layout tests: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/20384 Original change's description: > Fix "this" value in lazily-parsed module functions. > > When preparsing top-level functions in a module, we didn't track > unresolved variables. Consequently, "this" ended up referencing > the global "this", which has the wrong value (in a module "this" > is supposed to be the undefined value). > > This patch fixes that. This also lets us stop forcing context > allocation of all variables in module scopes, which the patch > takes care of as well. > > Bug: chromium:791334 > Change-Id: Ifac1f1adc033f3facfb3d29dd4bca32ee27bffcf > Reviewed-on: https://chromium-review.googlesource.com/808938 > Reviewed-by: Marja Hölttä <marja@chromium.org> > Reviewed-by: Adam Klein <adamk@chromium.org> > Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> > Commit-Queue: Georg Neis <neis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50025} TBR=adamk@chromium.org,marja@chromium.org,neis@chromium.org,kozyatinskiy@chromium.org Change-Id: I81f69334ed2ce104c00e6205d50001e4bdf07d15 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:791334 Reviewed-on: https://chromium-review.googlesource.com/822258 Reviewed-by: Michael Achenbach <machenbach@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#50036} [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/src/ast/scopes.cc [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/src/ast/scopes.h [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/src/parsing/parser.cc [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/cctest/interpreter/bytecode_expectations/Modules.golden [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/cctest/test-parsing.cc [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/debugger/debug/debug-modules-set-variable-value.js [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/debugger/debug/harmony/modules-debug-scopes2.js [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt [modify] https://crrev.com/62f09de9ab9a743cb929b327818b86d027cc888b/test/inspector/debugger/evaluate-on-call-frame-in-module.js [delete] https://crrev.com/10f392acf062cf5d4b23cf52a4fbfd3c8cad3b93/test/mjsunit/regress/regress-791334.js
,
Dec 12 2017
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/81f4fcd4347184fa32d440eae7d092a044d4f240 commit 81f4fcd4347184fa32d440eae7d092a044d4f240 Author: Georg Neis <neis@chromium.org> Date: Tue Dec 12 16:40:51 2017 [DevTools] prepare test before V8 roll With one of the next rolls, V8 will no longer force context allocation of module-scope variables. Bug: 791334 Change-Id: I93f38c190cc0ee68df4f2834083308b4504a8032 Reviewed-on: https://chromium-review.googlesource.com/822432 Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#523449} [modify] https://crrev.com/81f4fcd4347184fa32d440eae7d092a044d4f240/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/81f4fcd4347184fa32d440eae7d092a044d4f240/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger/debugger-es6-harmony-scopes-expected.txt
,
Dec 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/585b39f53a5eb3f4d931c505f3a1bc4288be60ce commit 585b39f53a5eb3f4d931c505f3a1bc4288be60ce Author: Georg Neis <neis@chromium.org> Date: Tue Dec 12 17:23:35 2017 Reland "Fix "this" value in lazily-parsed module functions." This is a reland of c3bd741efd29b8c6b322cea62f045e971cd7d183 Original change's description: > Fix "this" value in lazily-parsed module functions. > > When preparsing top-level functions in a module, we didn't track > unresolved variables. Consequently, "this" ended up referencing > the global "this", which has the wrong value (in a module "this" > is supposed to be the undefined value). > > This patch fixes that. This also lets us stop forcing context > allocation of all variables in module scopes, which the patch > takes care of as well. > > Bug: chromium:791334 > Change-Id: Ifac1f1adc033f3facfb3d29dd4bca32ee27bffcf > Reviewed-on: https://chromium-review.googlesource.com/808938 > Reviewed-by: Marja Hölttä <marja@chromium.org> > Reviewed-by: Adam Klein <adamk@chromium.org> > Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> > Commit-Queue: Georg Neis <neis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#50025} TBR=adamk@chromium.org TBR=kozyatinskiy@chromium.org Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Bug: chromium:791334 Change-Id: I57acc7b84a345565b36cbb55924fa2ff9b449eec Reviewed-on: https://chromium-review.googlesource.com/822341 Commit-Queue: Georg Neis <neis@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#50045} [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/src/ast/scopes.cc [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/src/ast/scopes.h [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/src/parsing/parser.cc [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/cctest/interpreter/bytecode_expectations/Modules.golden [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/cctest/test-parsing.cc [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/debugger/debug/debug-modules-set-variable-value.js [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/debugger/debug/harmony/modules-debug-scopes2.js [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt [modify] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/inspector/debugger/evaluate-on-call-frame-in-module.js [add] https://crrev.com/585b39f53a5eb3f4d931c505f3a1bc4288be60ce/test/mjsunit/regress/regress-791334.js
,
Dec 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4af8ffa951217528033e43164d978a79224866df commit 4af8ffa951217528033e43164d978a79224866df Author: Georg Neis <neis@chromium.org> Date: Tue Dec 19 08:13:27 2017 [DevTools] reenable debugger-es6-harmony-scopes.js test The relevant V8 change has now rolled into chromium, so the test can be enabled again. Bug: 791334 Change-Id: I767a31d831824d68ed07de8bc9168bdfcfd5290e Reviewed-on: https://chromium-review.googlesource.com/828953 Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#524969} [modify] https://crrev.com/4af8ffa951217528033e43164d978a79224866df/third_party/WebKit/LayoutTests/TestExpectations
,
Dec 19 2017
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by dtapu...@chromium.org
, Dec 3 2017