Revision: 22298
Author: [email protected]
Date: Wed Jul 9 11:35:05 2014 UTC
Log: Fix several issues with ES6 redeclaration checks
[email protected]
BUG=v8:3426
LOG=Y
Review URL: https://codereview.chromium.org/377513006
http://code.google.com/p/v8/source/detail?r=22298
Added:
/branches/bleeding_edge/test/mjsunit/harmony/regress/regress-3426.js
Modified:
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/test/mjsunit/harmony/block-conflicts.js
/branches/bleeding_edge/test/mjsunit/harmony/block-let-declaration.js
/branches/bleeding_edge/tools/generate-runtime-tests.py
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/harmony/regress/regress-3426.js
Wed Jul 9 11:35:05 2014 UTC
@@ -0,0 +1,7 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-scoping
+
+assertThrows("(function() { 'use strict'; { let f; var f; } })",
SyntaxError);
=======================================
--- /branches/bleeding_edge/src/parser.cc Wed Jul 9 07:50:11 2014 UTC
+++ /branches/bleeding_edge/src/parser.cc Wed Jul 9 11:35:05 2014 UTC
@@ -1709,15 +1709,14 @@
// Declare the name.
var = declaration_scope->DeclareLocal(
name, mode, declaration->initialization(), proxy->interface());
- } else if ((mode != VAR || var->mode() != VAR) &&
- (!declaration_scope->is_global_scope() ||
- IsLexicalVariableMode(mode) ||
- IsLexicalVariableMode(var->mode()))) {
+ } else if (IsLexicalVariableMode(mode) ||
IsLexicalVariableMode(var->mode())
+ || ((mode == CONST_LEGACY || var->mode() == CONST_LEGACY) &&
+ !declaration_scope->is_global_scope())) {
// The name was declared in this scope before; check for conflicting
// re-declarations. We have a conflict if either of the declarations
is
// not a var (in the global scope, we also have to ignore legacy
const for
// compatibility). There is similar code in runtime.cc in the Declare
- // functions. The function CheckNonConflictingScope checks for
conflicting
+ // functions. The function CheckConflictingVarDeclarations checks for
// var and let bindings from different scopes whereas this is a
check for
// conflicting declarations within the same scope. This check also
covers
// the special case
@@ -1900,11 +1899,12 @@
// Even if we're not at the top-level of the global or a function
// scope, we treat it as such and introduce the function with its
// initial value upon entering the corresponding scope.
- // In extended mode, a function behaves as a lexical binding, except in
the
- // global scope.
+ // In ES6, a function behaves as a lexical binding, except in the
+ // global scope, or the initial scope of eval or another function.
VariableMode mode =
- allow_harmony_scoping() &&
- strict_mode() == STRICT && !scope_->is_global_scope() ? LET : VAR;
+ allow_harmony_scoping() && strict_mode() == STRICT &&
+ !(scope_->is_global_scope() || scope_->is_eval_scope() ||
+ scope_->is_function_scope()) ? LET : VAR;
VariableProxy* proxy = NewUnresolved(name, mode, Interface::NewValue());
Declaration* declaration =
factory()->NewFunctionDeclaration(proxy, mode, fun, scope_, pos);
@@ -2213,9 +2213,8 @@
// executed.
//
// Executing the variable declaration statement will always
- // guarantee to give the global object a "local" variable; a
- // variable defined in the global object and not in any
- // prototype. This way, global variable declarations can shadow
+ // guarantee to give the global object an own property.
+ // This way, global variable declarations can shadow
// properties in the prototype chain, but only after the variable
// declaration statement has been executed. This is important in
// browsers where the global object (window) has lots of
@@ -3577,10 +3576,10 @@
}
ast_properties = *factory()->visitor()->ast_properties();
dont_optimize_reason = factory()->visitor()->dont_optimize_reason();
- }
- if (allow_harmony_scoping() && strict_mode() == STRICT) {
- CheckConflictingVarDeclarations(scope, CHECK_OK);
+ if (allow_harmony_scoping() && strict_mode() == STRICT) {
+ CheckConflictingVarDeclarations(scope, CHECK_OK);
+ }
}
FunctionLiteral::IsGeneratorFlag generator = is_generator
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/block-conflicts.js Tue Jun
17 15:22:38 2014 UTC
+++ /branches/bleeding_edge/test/mjsunit/harmony/block-conflicts.js Wed
Jul 9 11:35:05 2014 UTC
@@ -37,11 +37,20 @@
string.indexOf("redeclaration") >= 0);
return 'Conflict';
}
+
+
+function TestGlobal(s,e) {
+ try {
+ return eval(s + e);
+ } catch (x) {
+ return CheckException(x);
+ }
+}
function TestFunction(s,e) {
try {
- return eval("(function(){" + s + ";return " + e + "})")();
+ return eval("(function(){" + s + " return " + e + "})")();
} catch (x) {
return CheckException(x);
}
@@ -50,7 +59,7 @@
function TestBlock(s,e) {
try {
- return eval("(function(){ if (true) { " + s + "; }; return " + e
+ "})")();
+ return eval("(function(){ {" + s + "} return " + e + "})")();
} catch (x) {
return CheckException(x);
}
@@ -59,76 +68,123 @@
function TestAll(expected,s,opt_e) {
var e = "";
var msg = s;
- if (opt_e) { e = opt_e; msg += "; " + opt_e; }
- assertEquals(expected, TestFunction(s,e), "function:'" + msg + "'");
- assertEquals(expected, TestBlock(s,e), "block:'" + msg + "'");
+ if (opt_e) { e = opt_e; msg += opt_e; }
+ assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected,
+ TestGlobal(s,e), "global:'" + msg + "'");
+ assertEquals(expected === 'LocalConflict' ? 'NoConflict' : expected,
+ TestFunction(s,e), "function:'" + msg + "'");
+ assertEquals(expected === 'LocalConflict' ? 'Conflict' : expected,
+ TestBlock(s,e), "block:'" + msg + "'");
}
function TestConflict(s) {
TestAll('Conflict', s);
- TestAll('Conflict', 'eval("' + s + '")');
+ TestAll('Conflict', 'eval("' + s + '");');
}
-
function TestNoConflict(s) {
TestAll('NoConflict', s, "'NoConflict'");
- TestAll('NoConflict', 'eval("' + s + '")', "'NoConflict'");
+ TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
}
-var letbinds = [ "let x",
- "let x = 0",
- "let x = undefined",
- "function x() { }",
- "let x = function() {}",
- "let x, y",
- "let y, x",
- "const x = 0",
- "const x = undefined",
- "const x = function() {}",
- "const x = 2, y = 3",
- "const y = 4, x = 5",
+function TestLocalConflict(s) {
+ TestAll('LocalConflict', s, "'NoConflict'");
+ TestAll('NoConflict', 'eval("' + s + '");', "'NoConflict'");
+}
+
+var letbinds = [ "let x;",
+ "let x = 0;",
+ "let x = undefined;",
+ "let x = function() {};",
+ "let x, y;",
+ "let y, x;",
+ "const x = 0;",
+ "const x = undefined;",
+ "const x = function() {};",
+ "const x = 2, y = 3;",
+ "const y = 4, x = 5;",
];
-var varbinds = [ "var x",
- "var x = 0",
- "var x = undefined",
- "var x = function() {}",
- "var x, y",
- "var y, x",
+var varbinds = [ "var x;",
+ "var x = 0;",
+ "var x = undefined;",
+ "var x = function() {};",
+ "var x, y;",
+ "var y, x;",
];
-
+var funbind = "function x() {}";
for (var l = 0; l < letbinds.length; ++l) {
// Test conflicting let/var bindings.
for (var v = 0; v < varbinds.length; ++v) {
// Same level.
- TestConflict(letbinds[l] +'; ' + varbinds[v]);
- TestConflict(varbinds[v] +'; ' + letbinds[l]);
+ TestConflict(letbinds[l] + varbinds[v]);
+ TestConflict(varbinds[v] + letbinds[l]);
// Different level.
- TestConflict(letbinds[l] +'; {' + varbinds[v] + '; }');
- TestConflict('{ ' + varbinds[v] +'; }' + letbinds[l]);
+ TestConflict(letbinds[l] + '{' + varbinds[v] + '}');
+ TestConflict('{' + varbinds[v] +'}' + letbinds[l]);
+ TestNoConflict(varbinds[v] + '{' + letbinds[l] + '}');
+ TestNoConflict('{' + letbinds[l] + '}' + varbinds[v]);
+ // For loop.
+ TestConflict('for (' + letbinds[l] + '0;) {' + varbinds[v] + '}');
+ TestNoConflict('for (' + varbinds[v] + '0;) {' + letbinds[l] + '}');
}
// Test conflicting let/let bindings.
for (var k = 0; k < letbinds.length; ++k) {
// Same level.
- TestConflict(letbinds[l] +'; ' + letbinds[k]);
- TestConflict(letbinds[k] +'; ' + letbinds[l]);
+ TestConflict(letbinds[l] + letbinds[k]);
+ TestConflict(letbinds[k] + letbinds[l]);
// Different level.
- TestNoConflict(letbinds[l] +'; { ' + letbinds[k] + '; }');
- TestNoConflict('{ ' + letbinds[k] +'; } ' + letbinds[l]);
+ TestNoConflict(letbinds[l] + '{ ' + letbinds[k] + '}');
+ TestNoConflict('{' + letbinds[k] +'} ' + letbinds[l]);
+ // For loop.
+ TestNoConflict('for (' + letbinds[l] + '0;) {' + letbinds[k] + '}');
+ TestNoConflict('for (' + letbinds[k] + '0;) {' + letbinds[l] + '}');
}
+
+ // Test conflicting function/let bindings.
+ // Same level.
+ TestConflict(letbinds[l] + funbind);
+ TestConflict(funbind + letbinds[l]);
+ // Different level.
+ TestNoConflict(letbinds[l] + '{' + funbind + '}');
+ TestNoConflict('{' + funbind + '}' + letbinds[l]);
+ TestNoConflict(funbind + '{' + letbinds[l] + '}');
+ TestNoConflict('{' + letbinds[l] + '}' + funbind);
+ // For loop.
+ TestNoConflict('for (' + letbinds[l] + '0;) {' + funbind + '}');
// Test conflicting parameter/let bindings.
- TestConflict('(function (x) { ' + letbinds[l] + '; })()');
+ TestConflict('(function(x) {' + letbinds[l] + '})();');
+}
+
+// Test conflicting function/var bindings.
+for (var v = 0; v < varbinds.length; ++v) {
+ // Same level.
+ TestLocalConflict(varbinds[v] + funbind);
+ TestLocalConflict(funbind + varbinds[v]);
+ // Different level.
+ TestLocalConflict(funbind + '{' + varbinds[v] + '}');
+ TestLocalConflict('{' + varbinds[v] +'}' + funbind);
+ TestNoConflict(varbinds[v] + '{' + funbind + '}');
+ TestNoConflict('{' + funbind + '}' + varbinds[v]);
+ // For loop.
+ TestNoConflict('for (' + varbinds[v] + '0;) {' + funbind + '}');
}
// Test conflicting catch/var bindings.
for (var v = 0; v < varbinds.length; ++v) {
- TestConflict('try {} catch (x) { ' + varbinds[v] + '; }');
+ TestConflict('try {} catch(x) {' + varbinds[v] + '}');
}
// Test conflicting parameter/var bindings.
for (var v = 0; v < varbinds.length; ++v) {
- TestNoConflict('(function (x) { ' + varbinds[v] + '; })()');
+ TestNoConflict('(function (x) {' + varbinds[v] + '})();');
}
+
+// Test conflicting catch/function bindings.
+TestNoConflict('try {} catch(x) {' + funbind + '}');
+
+// Test conflicting parameter/function bindings.
+TestNoConflict('(function (x) {' + funbind + '})();');
=======================================
--- /branches/bleeding_edge/test/mjsunit/harmony/block-let-declaration.js
Tue Jun 17 15:22:38 2014 UTC
+++ /branches/bleeding_edge/test/mjsunit/harmony/block-let-declaration.js
Wed Jul 9 11:35:05 2014 UTC
@@ -56,11 +56,11 @@
// an exception in eval code during parsing, before even compiling or
executing
// the code. Thus the generated function is not called here.
function TestLocalThrows(str, expect) {
- assertThrows("(function(){ 'use strict'; " + str + "})", expect);
+ assertThrows("(function(arg){ 'use strict'; " + str + "})", expect);
}
function TestLocalDoesNotThrow(str) {
- assertDoesNotThrow("(function(){ 'use strict'; " + str + "})()");
+ assertDoesNotThrow("(function(arg){ 'use strict'; " + str + "})()");
}
// Test let declarations in statement positions.
@@ -108,6 +108,28 @@
TestLocalDoesNotThrow("switch (true) { case true: var x; }");
TestLocalDoesNotThrow("switch (true) { default: var x; }");
+// Test that redeclarations of functions are only allowed in outermost
scope.
+TestLocalThrows("{ let f; var f; }");
+TestLocalThrows("{ var f; let f; }");
+TestLocalThrows("{ function f() {} let f; }");
+TestLocalThrows("{ let f; function f() {} }");
+TestLocalThrows("{ function f() {} var f; }");
+TestLocalThrows("{ var f; function f() {} }");
+TestLocalThrows("{ function f() {} function f() {} }");
+TestLocalThrows("function f() {} let f;");
+TestLocalThrows("let f; function f() {}");
+TestLocalDoesNotThrow("function arg() {}");
+TestLocalDoesNotThrow("function f() {} var f;");
+TestLocalDoesNotThrow("var f; function f() {}");
+TestLocalDoesNotThrow("function f() {} function f() {}");
+
+function g(f) {
+ function f() { return 1 }
+ return f()
+}
+assertEquals(1, g(function() { return 2 }))
+
+
// Test function declarations in source element and
// sloppy statement positions.
function f() {
=======================================
--- /branches/bleeding_edge/tools/generate-runtime-tests.py Thu Jul 3
15:56:01 2014 UTC
+++ /branches/bleeding_edge/tools/generate-runtime-tests.py Wed Jul 9
11:35:05 2014 UTC
@@ -51,7 +51,7 @@
EXPECTED_FUZZABLE_COUNT = 332
EXPECTED_CCTEST_COUNT = 6
EXPECTED_UNKNOWN_COUNT = 4
-EXPECTED_BUILTINS_COUNT = 810
+EXPECTED_BUILTINS_COUNT = 815
# Don't call these at all.
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.