From 23235b5fdb5412355e826f7fc0c154c409b9be37 Mon Sep 17 00:00:00 2001 From: littledan Date: Fri, 8 Jan 2016 09:34:36 -0800 Subject: [PATCH] Reland of Ship ES2015 sloppy-mode function hoisting, let, class (patchset #1 id:1 of https://codereview.chromium.org/1565263002/ ) Reason for revert: Crash fixed by https://codereview.chromium.org/1564923007 Original issue's description: > Revert of Ship ES2015 sloppy-mode function hoisting, let, class (patchset #7 id:120001 of https://codereview.chromium.org/1551443002/ ) > > Reason for revert: > Causes frequent crashes in Canary: chromium:537816 > > Original issue's description: > > Ship ES2015 sloppy-mode function hoisting, let, class > > > > This patch doesn't ship all features of ES2015 variable/scoping > > changes, notably omitting the removal of legacy const. I think > > function hoisting, let and class in sloppy mode can stand to > > themselves as a package, and the legacy const change is much > > riskier and more likely to be reverted, so my intention is to > > pursue those as a separate, follow-on patch. > > > > R=adamk@chromium.org > > BUG=v8:4285,v8:3305 > > LOG=Y > > CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng;tryserver.blink:linux_blink_rel > > > > Committed: https://crrev.com/fcff8588a5a01587643d6c2507c7b882c78a2957 > > Cr-Commit-Position: refs/heads/master@{#33133} > > TBR=adamk@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG=v8:4285,v8:3305,chromium:537816 > LOG=Y > > Committed: https://crrev.com/adac5956c6216056a211cfaa460a00ac1500d8f8 > Cr-Commit-Position: refs/heads/master@{#33162} TBR=adamk@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4285,v8:3305,chromium:537816 Review URL: https://codereview.chromium.org/1571793002 Cr-Commit-Position: refs/heads/master@{#33189} --- src/flag-definitions.h | 8 ++++---- test/message/no-legacy-const-2.js | 3 ++- test/message/no-legacy-const-2.out | 2 +- test/message/no-legacy-const-3.js | 3 ++- test/message/no-legacy-const-3.out | 2 +- test/message/no-legacy-const.js | 3 ++- test/message/no-legacy-const.out | 2 +- test/mjsunit/es6/block-early-errors.js | 2 ++ .../block-eval-var-over-legacy-const.js | 0 .../block-let-contextual-sloppy.js | 0 test/mjsunit/es6/block-non-strict-errors.js | 3 +++ .../{harmony => es6}/block-scope-class.js | 0 .../classes-derived-return-type.js | 0 test/mjsunit/harmony/destructuring.js | 5 +++-- test/mjsunit/mjsunit.status | 4 ++++ test/mjsunit/regress/regress-91120.js | 20 ++++++++++--------- test/mjsunit/regress/regress-crbug-323936.js | 8 +++++--- .../regress/regress-crbug-451770.js | 0 test/mozilla/mozilla.status | 13 +++--------- .../webkit/fast/js/kde/func-decl-expected.txt | 9 ++++++--- test/webkit/webkit.status | 3 +++ 21 files changed, 53 insertions(+), 37 deletions(-) rename test/mjsunit/{harmony => es6}/block-eval-var-over-legacy-const.js (100%) rename test/mjsunit/{harmony => es6}/block-let-contextual-sloppy.js (100%) rename test/mjsunit/{harmony => es6}/block-scope-class.js (100%) rename test/mjsunit/{harmony => es6}/classes-derived-return-type.js (100%) rename test/mjsunit/{harmony => }/regress/regress-crbug-451770.js (100%) diff --git a/src/flag-definitions.h b/src/flag-definitions.h index db14c178d37..1af37c0b54f 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -212,9 +212,6 @@ DEFINE_IMPLICATION(es_staging, move_object_start) #define HARMONY_STAGED(V) \ V(harmony_proxies, "harmony proxies") \ V(harmony_reflect, "harmony Reflect API") \ - V(harmony_sloppy, "harmony features in sloppy mode") \ - V(harmony_sloppy_let, "harmony let in sloppy mode") \ - V(harmony_sloppy_function, "harmony sloppy function block scoping") \ V(harmony_regexp_lookbehind, "harmony regexp lookbehind") // Features that are shipping (turned on by default, but internal flag remains). @@ -227,7 +224,10 @@ DEFINE_IMPLICATION(es_staging, move_object_start) V(harmony_tolength, "harmony ToLength") \ V(harmony_tostring, "harmony toString") \ V(harmony_completion, "harmony completion value semantics") \ - V(harmony_regexps, "harmony regular expression extensions") + V(harmony_regexps, "harmony regular expression extensions") \ + V(harmony_sloppy, "harmony features in sloppy mode") \ + V(harmony_sloppy_let, "harmony let in sloppy mode") \ + V(harmony_sloppy_function, "harmony sloppy function block scoping") // Once a shipping feature has proved stable in the wild, it will be dropped diff --git a/test/message/no-legacy-const-2.js b/test/message/no-legacy-const-2.js index 29aeba5e1f1..24e3f856392 100644 --- a/test/message/no-legacy-const-2.js +++ b/test/message/no-legacy-const-2.js @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // -// Flags: --no-legacy-const +// Flags: --no-legacy-const --no-harmony-sloppy --no-harmony-sloppy-let +// Flags: --no-harmony-sloppy-function const = 42; diff --git a/test/message/no-legacy-const-2.out b/test/message/no-legacy-const-2.out index 55c855ee4f4..5385250aafa 100644 --- a/test/message/no-legacy-const-2.out +++ b/test/message/no-legacy-const-2.out @@ -1,4 +1,4 @@ -*%(basename)s:7: SyntaxError: Unexpected token const +*%(basename)s:8: SyntaxError: Unexpected token const const = 42; ^^^^^ diff --git a/test/message/no-legacy-const-3.js b/test/message/no-legacy-const-3.js index 6981571e622..4f6e9a4bbb3 100644 --- a/test/message/no-legacy-const-3.js +++ b/test/message/no-legacy-const-3.js @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // -// Flags: --no-legacy-const +// Flags: --no-legacy-const --no-harmony-sloppy --no-harmony-sloppy-let +// Flags: --no-harmony-sloppy-function const diff --git a/test/message/no-legacy-const-3.out b/test/message/no-legacy-const-3.out index 046e9f70236..7539bbcd1d8 100644 --- a/test/message/no-legacy-const-3.out +++ b/test/message/no-legacy-const-3.out @@ -1,4 +1,4 @@ -*%(basename)s:7: SyntaxError: Unexpected token const +*%(basename)s:8: SyntaxError: Unexpected token const const ^^^^^ diff --git a/test/message/no-legacy-const.js b/test/message/no-legacy-const.js index ecad2181b88..d9a716b3a88 100644 --- a/test/message/no-legacy-const.js +++ b/test/message/no-legacy-const.js @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // -// Flags: --no-legacy-const +// Flags: --no-legacy-const --no-harmony-sloppy --no-harmony-sloppy-let +// Flags: --no-harmony-sloppy-function const x = 42; diff --git a/test/message/no-legacy-const.out b/test/message/no-legacy-const.out index b28dd10b77b..33bb0388360 100644 --- a/test/message/no-legacy-const.out +++ b/test/message/no-legacy-const.out @@ -1,4 +1,4 @@ -*%(basename)s:7: SyntaxError: Unexpected token const +*%(basename)s:8: SyntaxError: Unexpected token const const x = 42; ^^^^^ diff --git a/test/mjsunit/es6/block-early-errors.js b/test/mjsunit/es6/block-early-errors.js index bf24942bb1c..4af6521f647 100644 --- a/test/mjsunit/es6/block-early-errors.js +++ b/test/mjsunit/es6/block-early-errors.js @@ -25,6 +25,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// Flags: --no-harmony-sloppy-let + function CheckException(e) { var string = e.toString(); assertInstanceof(e, SyntaxError); diff --git a/test/mjsunit/harmony/block-eval-var-over-legacy-const.js b/test/mjsunit/es6/block-eval-var-over-legacy-const.js similarity index 100% rename from test/mjsunit/harmony/block-eval-var-over-legacy-const.js rename to test/mjsunit/es6/block-eval-var-over-legacy-const.js diff --git a/test/mjsunit/harmony/block-let-contextual-sloppy.js b/test/mjsunit/es6/block-let-contextual-sloppy.js similarity index 100% rename from test/mjsunit/harmony/block-let-contextual-sloppy.js rename to test/mjsunit/es6/block-let-contextual-sloppy.js diff --git a/test/mjsunit/es6/block-non-strict-errors.js b/test/mjsunit/es6/block-non-strict-errors.js index 50d5f22cf14..db7f558905f 100644 --- a/test/mjsunit/es6/block-non-strict-errors.js +++ b/test/mjsunit/es6/block-non-strict-errors.js @@ -2,6 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// Flags: --no-harmony-sloppy-let --no-harmony-sloppy-function +// Flags: --no-harmony-sloppy + function CheckError(source) { var exception = null; try { diff --git a/test/mjsunit/harmony/block-scope-class.js b/test/mjsunit/es6/block-scope-class.js similarity index 100% rename from test/mjsunit/harmony/block-scope-class.js rename to test/mjsunit/es6/block-scope-class.js diff --git a/test/mjsunit/harmony/classes-derived-return-type.js b/test/mjsunit/es6/classes-derived-return-type.js similarity index 100% rename from test/mjsunit/harmony/classes-derived-return-type.js rename to test/mjsunit/es6/classes-derived-return-type.js diff --git a/test/mjsunit/harmony/destructuring.js b/test/mjsunit/harmony/destructuring.js index 984efb00163..50f27857ec0 100644 --- a/test/mjsunit/harmony/destructuring.js +++ b/test/mjsunit/harmony/destructuring.js @@ -991,8 +991,9 @@ function f20({x}) { function x() { return 2 }; return x(); } assertEquals(2, f20({x: 1})); + // Function hoisting is blocked by the conflicting x declaration function f21({x}) { { function x() { return 2 } } return x(); } - assertEquals(2, f21({x: 1})); + assertThrows(() => f21({x: 1}), TypeError); var g1 = ({x}) => { var x = 2; return x }; assertEquals(2, g1({x: 1})); @@ -1025,7 +1026,7 @@ var g20 = ({x}) => { function x() { return 2 }; return x(); } assertEquals(2, g20({x: 1})); var g21 = ({x}) => { { function x() { return 2 } } return x(); } - assertEquals(2, g21({x: 1})); + assertThrows(() => g21({x: 1}), TypeError); assertThrows("'use strict'; function f(x) { let x = 0; }; f({});", SyntaxError); assertThrows("'use strict'; function f({x}) { let x = 0; }; f({});", SyntaxError); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index b2a3e24bd2d..4c649fdf450 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -1135,6 +1135,10 @@ 'with-prototype': [SKIP], 'with-readonly': [SKIP], 'with-value': [SKIP], + 'regress/regress-builtinbust-7': [SKIP], + 'regress/regress-crbug-451770': [SKIP], + 'regress/regress-crbug-503968': [SKIP], + 'regress/regress-crbug-504729': [SKIP], }], # ignition == True ['ignition == True and (arch == arm or arch == arm64)', { diff --git a/test/mjsunit/regress/regress-91120.js b/test/mjsunit/regress/regress-91120.js index 117acac6cdc..73f545648af 100644 --- a/test/mjsunit/regress/regress-91120.js +++ b/test/mjsunit/regress/regress-91120.js @@ -25,24 +25,26 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// We intend that the function declaration for g inside catch is hoisted to -// function f's scope. Invoke it before try/catch, in the try block, in the -// catch block, after try/catch, and outside f, and verify that it has -// access to the proper binding of x. +// With ES2015 function hoisting semantics, functions are only "hoisted" out +// of blocks by writing their values into var-scoped declarations. Therefore, +// they access the catch binding when it syntactically appears so. +// This is a potentially breaking change vs the old semantics, which would +// return 'function' from g() everywhere. + var x = 'global'; function f() { var x = 'function'; - assertEquals('function', g()); + assertEquals(undefined, g); try { - assertEquals('function', g()); + assertEquals(undefined, g); throw 'catch'; } catch (x) { function g() { return x; } - assertEquals('function', g()); + assertEquals('catch', g()); } - assertEquals('function', g()); + assertEquals('catch', g()); return g; } -assertEquals('function', f()()); +assertEquals('catch', f()()); diff --git a/test/mjsunit/regress/regress-crbug-323936.js b/test/mjsunit/regress/regress-crbug-323936.js index c1d0f7d9311..6e75729c181 100644 --- a/test/mjsunit/regress/regress-crbug-323936.js +++ b/test/mjsunit/regress/regress-crbug-323936.js @@ -17,7 +17,7 @@ function listener(event, exec_state, event_data, data) { exec_state.frame(0).evaluate("write_0('foo')"); exec_state.frame(0).evaluate("write_1('modified')"); } else { - assertEquals("foo", exec_state.frame(0).evaluate("e").value()); + assertEquals("argument", exec_state.frame(0).evaluate("e").value()); exec_state.frame(0).evaluate("write_2('bar')"); } step++; @@ -33,12 +33,14 @@ function f(e, x) { try { throw "error"; } catch(e) { - // 'e' and 'x' bind to the argument due to hoisting + // In ES2015 hoisting semantics, 'x' binds to the argument + // and 'e' binds to the exception. function write_0(v) { e = v } function write_1(v) { x = v } debugger; - assertEquals("error", e); + assertEquals("foo", e); // overwritten by the debugger } + assertEquals("argument", e); // debugger did not overwrite function write_2(v) { e = v } debugger; assertEquals("bar", e); diff --git a/test/mjsunit/harmony/regress/regress-crbug-451770.js b/test/mjsunit/regress/regress-crbug-451770.js similarity index 100% rename from test/mjsunit/harmony/regress/regress-crbug-451770.js rename to test/mjsunit/regress/regress-crbug-451770.js diff --git a/test/mozilla/mozilla.status b/test/mozilla/mozilla.status index c1930f97f96..7e8effed01c 100644 --- a/test/mozilla/mozilla.status +++ b/test/mozilla/mozilla.status @@ -606,16 +606,6 @@ 'js1_5/Regress/regress-290575': [PASS, FAIL_OK], - # Fails because of the way function declarations are - # handled in V8/JSC. V8 follows IE behavior and introduce - # all nested function declarations when entering the - # surrounding function, whereas Spidermonkey declares - # them dynamically when the statement is executed. - 'ecma_3/Function/scope-001': [FAIL_OK], - 'ecma_3/FunExpr/fe-001': [FAIL_OK], - 'js1_5/Scope/regress-184107': [FAIL_OK], - - # Function is deletable in V8 and JSC. 'js1_5/Regress/regress-352604': [FAIL_OK], @@ -673,6 +663,9 @@ # We do not correctly handle assignments within "with" 'ecma_3/Statements/12.10-01': [FAIL], + # https://bugs.chromium.org/p/v8/issues/detail?id=4647 + 'ecma_3/FunExpr/fe-001': [FAIL_OK], + ##################### MOZILLA EXTENSION TESTS ##################### 'ecma/extensions/15.1.2.1-1': [FAIL_OK], diff --git a/test/webkit/fast/js/kde/func-decl-expected.txt b/test/webkit/fast/js/kde/func-decl-expected.txt index d2db3810d93..d5b8a6fa101 100644 --- a/test/webkit/fast/js/kde/func-decl-expected.txt +++ b/test/webkit/fast/js/kde/func-decl-expected.txt @@ -21,21 +21,24 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# Due to changes in ES2015 function hoisting semantics, this test is +# no longer really accurate and is expected to fail. test262 and mjsunit +# tests verify the correct semantics. KDE JS Test On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS Function declaration takes effect at entry +FAIL Function declaration takes effect at entry: value has type undefined , not:function PASS Decl not yet overwritten PASS After assign (0) PASS function decls have no execution content PASS After assign #2 (0) -PASS Decl already overwritten +FAIL Decl already overwritten: value has type function , not:number PASS After assign (1) PASS function decls have no execution content PASS After assign #2 (1) -PASS Decl already overwritten +FAIL Decl already overwritten: value has type function , not:number PASS After assign (2) PASS function decls have no execution content PASS After assign #2 (2) diff --git a/test/webkit/webkit.status b/test/webkit/webkit.status index 6c3f402d63c..971cf4691f9 100644 --- a/test/webkit/webkit.status +++ b/test/webkit/webkit.status @@ -35,6 +35,9 @@ 'dfg-inline-arguments-reset-changetype': [PASS, FAIL], # TODO(turbofan): We run out of stack earlier on 64-bit for now. 'fast/js/deep-recursion-test': [PASS, NO_VARIANTS], + # This test leads to a SyntaxError from conflicting let declarations + # in ES2015 + 'function-declarations-in-switch-statement': [FAIL], }], # ALWAYS ['mode == debug', { # Too slow in debug mode.