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.

Reply via email to