Add errors for declarations which conflict with catch parameters.

Catch parameters are largely treated as lexical declarations in the
block which contains their body for the purposes of early syntax errors,
with some exceptions outlined in B.3.5. This patch introduces most of
those errors, except those from `eval('for (var e of ...);')` inside of
a catch with a simple parameter named 'e'.

Note that annex B.3.5 allows var declarations to conflict with simple
catch parameters, except when the variable declaration is the init of a
for-of statement.

BUG=v8:5112,v8:4231

Review-Url: https://codereview.chromium.org/2109733003
Cr-Commit-Position: refs/heads/master@{#37462}

src/ast/scopes.cc

@@ -141,7 +141,7 @@ Scope::Scope(Zone* zone, Scope* inner_scope,
       zone_(zone) {
   SetDefaults(CATCH_SCOPE, NULL, Handle<ScopeInfo>::null());
   AddInnerScope(inner_scope);
-  ++num_var_or_const_;
+  ++num_var_;
   num_heap_slots_ = Context::MIN_CONTEXT_SLOTS;
   Variable* variable = variables_.Declare(this,
                                           catch_variable_name,
@@ -185,7 +185,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
   force_eager_compilation_ = false;
   force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
       ? outer_scope->has_forced_context_allocation() : false;
-  num_var_or_const_ = 0;
+  num_var_ = 0;
   num_stack_slots_ = 0;
   num_heap_slots_ = 0;
   num_global_slots_ = 0;
@@ -339,8 +339,7 @@ Scope* Scope::FinalizeBlockScope() {
   DCHECK(temps_.is_empty());
   DCHECK(params_.is_empty());
 
-  if (num_var_or_const() > 0 ||
-      (is_declaration_scope() && calls_sloppy_eval())) {
+  if (num_var() > 0 || (is_declaration_scope() && calls_sloppy_eval())) {
     return this;
   }
 
@@ -512,7 +511,7 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   // introduced during variable allocation, and TEMPORARY variables are
   // allocated via NewTemporary().
   DCHECK(IsDeclaredVariableMode(mode));
-  ++num_var_or_const_;
+  ++num_var_;
   return variables_.Declare(this, name, mode, kind, init_flag,
                             maybe_assigned_flag);
 }
@@ -611,6 +610,25 @@ Declaration* Scope::CheckConflictingVarDeclarations() {
   return NULL;
 }
 
+Declaration* Scope::CheckLexDeclarationsConflictingWith(
+    ZoneList<const AstRawString*>* names) {
+  int length = names->length();
+  for (int i = 0; i < length; ++i) {
+    Variable* var = LookupLocal(names->at(i));
+    if (var != nullptr && IsLexicalVariableMode(var->mode())) {
+      // Conflict; find and return its declaration.
+      const AstRawString* name = names->at(i);
+      int decls_length = decls_.length();
+      for (int j = 0; j < decls_length; ++j) {
+        if (decls_[i]->proxy()->raw_name() == name) {
+          return decls_[i];
+        }
+      }
+      DCHECK(false);
+    }
+  }
+  return nullptr;
+}
 
 class VarAndOrder {
  public:

src/ast/scopes.h

@@ -236,6 +236,14 @@ class Scope: public ZoneObject {
   // scope over a let binding of the same name.
   Declaration* CheckConflictingVarDeclarations();
 
+  // Check if the scope has a conflicting lexical declaration that has a name in
+  // the given list. This is used to catch patterns like
+  // `try{}catch(e){let e;}`,
+  // which is an error even though the two 'e's are declared in different
+  // scopes.
+  Declaration* CheckLexDeclarationsConflictingWith(
+      ZoneList<const AstRawString*>* names);
+
   // ---------------------------------------------------------------------------
   // Scope-specific info.
 
@@ -491,6 +499,12 @@ class Scope: public ZoneObject {
   // The ModuleDescriptor for this scope; only for module scopes.
   ModuleDescriptor* module() const { return module_descriptor_; }
 
+  AstRawString* catch_variable_name() const {
+    DCHECK(is_catch_scope());
+    DCHECK(num_var() == 1);
+    return static_cast<AstRawString*>(variables_.Start()->key);
+  }
+
   // ---------------------------------------------------------------------------
   // Variable allocation.
 
@@ -501,8 +515,8 @@ class Scope: public ZoneObject {
                                     ZoneList<Variable*>* context_locals,
                                     ZoneList<Variable*>* context_globals);
 
-  // Current number of var or const locals.
-  int num_var_or_const() { return num_var_or_const_; }
+  // Current number of var locals.
+  int num_var() const { return num_var_; }
 
   // Result of variable allocation.
   int num_stack_slots() const { return num_stack_slots_; }
@@ -673,7 +687,7 @@ class Scope: public ZoneObject {
   bool is_declaration_scope_;
 
   // Computed as variables are declared.
-  int num_var_or_const_;
+  int num_var_;
 
   // Computed via AllocateVariables; function, block and catch scopes only.
   int num_stack_slots_;

src/parsing/pattern-rewriter.cc

@@ -164,7 +164,7 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   Scope* declaration_scope = IsLexicalVariableMode(descriptor_->mode)
                                  ? descriptor_->scope
                                  : descriptor_->scope->DeclarationScope();
-  if (declaration_scope->num_var_or_const() > kMaxNumFunctionLocals) {
+  if (declaration_scope->num_var() > kMaxNumFunctionLocals) {
     parser_->ReportMessage(MessageTemplate::kTooManyVariables);
     *ok_ = false;
     return;

test/mjsunit/es6/block-conflicts-sloppy.js

@@ -170,8 +170,5 @@ for (var v = 0; v < varbinds.length; ++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 + '})();');

test/mjsunit/es6/block-conflicts.js

@@ -170,8 +170,5 @@ for (var v = 0; v < varbinds.length; ++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 + '})();');

test/mjsunit/es6/block-sloppy-function.js

@@ -455,6 +455,45 @@
   assertEquals(4, f());
 })();
 
+// B.3.5 interacts with B.3.3 to allow this.
+(function hoistingThroughSimpleCatch() {
+  assertEquals(undefined, f);
+
+  try {
+    throw 0;
+  } catch(f) {
+    {
+      assertEquals(4, f());
+
+      function f() {
+        return 4;
+      }
+
+      assertEquals(4, f());
+    }
+  }
+
+  assertEquals(4, f());
+})();
+
+(function noHoistingThroughComplexCatch() {
+  try {
+    throw 0;
+  } catch({f}) {
+    {
+      assertEquals(4, f());
+
+      function f() {
+        return 4;
+      }
+
+      assertEquals(4, f());
+    }
+  }
+
+  assertThrows(()=>f, ReferenceError);
+})();
+
 // Test that hoisting from blocks does happen in global scope
 function globalHoisted() { return 0; }
 {

test/mjsunit/es6/catch-parameter-redeclaration.js

+// Copyright 2016 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.
+
+function checkIsRedeclarationError(code) {
+  try {
+    eval(`
+checkIsRedeclarationError : {
+  break checkIsRedeclarationError;
+${code}
+}
+`);
+    assertUnreachable();
+  } catch(e) {
+    assertInstanceof(e, SyntaxError );
+    assertTrue( e.toString().indexOf("has already been declared") >= 0 );
+  }
+}
+
+function checkIsNotRedeclarationError(code) {
+  assertDoesNotThrow(()=>eval(`
+checkIsNotRedeclarationError_label : {
+  break checkIsNotRedeclarationError_label;
+${code}
+}
+`));
+}
+
+
+let lexical_e = [
+  'let e',
+  'let {e} = 0',
+  'let [e] = 0',
+  'let {f:e} = 0',
+  'let [[[], e]] = 0',
+  'const e = 0',
+  'const {e} = 0',
+  'const [e] = 0',
+  'const {f:e} = 0',
+  'const [[[], e]] = 0',
+  'function e(){}',
+  'function* e(){}',
+];
+
+let not_lexical_e = [
+  'var e',
+  'var {e} = 0',
+  'let {} = 0',
+  'let {e:f} = 0',
+  '{ function e(){} }'
+];
+
+// Check that lexical declarations cannot override a simple catch parameter
+for (let declaration of lexical_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  ${declaration}
+}
+`);
+}
+
+// Check that lexical declarations cannot override a complex catch parameter
+for (let declaration of lexical_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch({e}) {
+  ${declaration}
+}
+`);
+}
+
+// Check that non-lexical declarations can override a simple catch parameter
+for (let declaration of not_lexical_e) {
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  ${declaration}
+}
+`);
+}
+
+// Check that the above error does not occur if a declaration scope is between
+// the catch and the loop.
+for (let declaration of lexical_e) {
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  (()=>{${declaration}})();
+}
+`);
+
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  (function(){${declaration}})();
+}
+`);
+}

test/mjsunit/es6/for-each-in-catch.js

+// Copyright 2016 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.
+
+function checkIsRedeclarationError(code) {
+  try {
+    eval(`
+checkIsRedeclarationError : {
+  break checkIsRedeclarationError;
+${code}
+}
+`);
+    assertUnreachable();
+  } catch(e) {
+    assertInstanceof(e, SyntaxError );
+    assertTrue( e.toString().indexOf("has already been declared") >= 0 );
+  }
+}
+
+function checkIsNotRedeclarationError(code) {
+  assertDoesNotThrow(()=>eval(`
+checkIsNotRedeclarationError_label : {
+  break checkIsNotRedeclarationError_label;
+${code}
+}
+`));
+}
+
+
+let var_e = [
+  'var e',
+  'var {e}',
+  'var [e]',
+  'var {f:e}',
+  'var [[[], e]]'
+];
+
+let not_var_e = [
+  'var f',
+  'var {}',
+  'var {e:f}',
+  'e',
+  '{e}',
+  'let e',
+  'const e',
+  'let {e}',
+  'const {e}',
+  'let [e]',
+  'const [e]',
+  'let {f:e}',
+  'const {f:e}'
+];
+
+// Check that `for (var ... of ...)` cannot redeclare a simple catch variable
+// but `for (var ... in ...)` can.
+for (let binding of var_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  for (${binding} of []);
+}
+`);
+
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  for (${binding} in []);
+}
+`);
+}
+
+// Check that the above error occurs even for nested catches.
+for (let binding of var_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  try {
+    throw 1;
+  } catch(f) {
+    try {
+      throw 2;
+    } catch({}) {
+      for (${binding} of []);
+    }
+  }
+}
+`);
+
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  try {
+    throw 1;
+  } catch(f) {
+    try {
+      throw 2;
+    } catch({}) {
+      for (${binding} in []);
+    }
+  }
+}
+`);
+}
+
+// Check that the above error does not occur if a declaration scope is between
+// the catch and the loop.
+for (let binding of var_e) {
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  (()=>{for (${binding} of []);})();
+}
+`);
+
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  (function(){for (${binding} of []);})();
+}
+`);
+}
+
+// Check that there is no error when not declaring a var named e.
+for (let binding of not_var_e) {
+  checkIsNotRedeclarationError(`
+try {
+  throw 0;
+} catch(e) {
+  for (${binding} of []);
+}
+`);
+}
+
+// Check that there is an error for both for-in and for-of when redeclaring
+// a non-simple catch parameter
+for (let binding of var_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch({e}) {
+  for (${binding} of []);
+}
+`);
+
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch({e}) {
+  for (${binding} in []);
+}
+`);
+}
+
+// Check that the above error occurs even for nested catches.
+for (let binding of var_e) {
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch({e}) {
+  try {
+    throw 1;
+  } catch(f) {
+    try {
+      throw 2;
+    } catch({}) {
+      for (${binding} of []);
+    }
+  }
+}
+`);
+
+  checkIsRedeclarationError(`
+try {
+  throw 0;
+} catch({e}) {
+  try {
+    throw 1;
+  } catch(f) {
+    try {
+      throw 2;
+    } catch({}) {
+      for (${binding} in []);
+    }
+  }
+}
+`);
+}

test/test262/test262.status

@@ -245,8 +245,6 @@
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=4231
   'language/eval-code/direct/var-env-lower-lex-catch-non-strict': [FAIL],
-  'language/statements/try/early-catch-lex': [FAIL],
-  'language/statements/try/early-catch-var': [FAIL],
 
   # https://bugs.chromium.org/p/v8/issues/detail?id=4951
   'language/expressions/assignment/dstr-array-elem-iter-rtrn-close': [FAIL],
@@ -331,9 +329,6 @@
   # https://bugs.chromium.org/p/v8/issues/detail?id=1569
   'language/module-code/*': [SKIP],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=5112
-  'language/statements/try/scope-catch-block-lex-open': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=5012
   'intl402/Intl/getCanonicalLocales/*': [FAIL],