Skip to content

Commit

Permalink
Reland of Ship ES2015 sloppy-mode function hoisting, let, class (patc…
Browse files Browse the repository at this point in the history
…hset crosswalk-project#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 crosswalk-project#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}
  • Loading branch information
littledan authored and Commit bot committed Jan 8, 2016
1 parent 3f0b6c5 commit 23235b5
Show file tree
Hide file tree
Showing 21 changed files with 53 additions and 37 deletions.
8 changes: 4 additions & 4 deletions src/flag-definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/message/no-legacy-const-2.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion test/message/no-legacy-const-2.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:7: SyntaxError: Unexpected token const
*%(basename)s:8: SyntaxError: Unexpected token const
const = 42;
^^^^^

Expand Down
3 changes: 2 additions & 1 deletion test/message/no-legacy-const-3.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/message/no-legacy-const-3.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:7: SyntaxError: Unexpected token const
*%(basename)s:8: SyntaxError: Unexpected token const
const
^^^^^

Expand Down
3 changes: 2 additions & 1 deletion test/message/no-legacy-const.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion test/message/no-legacy-const.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
*%(basename)s:7: SyntaxError: Unexpected token const
*%(basename)s:8: SyntaxError: Unexpected token const
const x = 42;
^^^^^

Expand Down
2 changes: 2 additions & 0 deletions test/mjsunit/es6/block-early-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions test/mjsunit/es6/block-non-strict-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
File renamed without changes.
5 changes: 3 additions & 2 deletions test/mjsunit/harmony/destructuring.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}));
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions test/mjsunit/mjsunit.status
Original file line number Diff line number Diff line change
Expand Up @@ -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)', {
Expand Down
20 changes: 11 additions & 9 deletions test/mjsunit/regress/regress-91120.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()());
8 changes: 5 additions & 3 deletions test/mjsunit/regress/regress-crbug-323936.js
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand All @@ -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);
Expand Down
13 changes: 3 additions & 10 deletions test/mozilla/mozilla.status
Original file line number Diff line number Diff line change
Expand Up @@ -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],

Expand Down Expand Up @@ -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],
Expand Down
9 changes: 6 additions & 3 deletions test/webkit/fast/js/kde/func-decl-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions test/webkit/webkit.status
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 23235b5

Please sign in to comment.