[parser] Change how catch is parsed

- Directly declares the special catch variable from the parser-base.
- Tracks Scope on PreParserBlock and finds conflicting lexical declarations by
  simply walking the VariableMap of the block inserted for the pattern; or the
  catch variable in case of identifier.
- This also enables throwing errors for duplicate let in the preparser. We may
  have to back that out if it breaks something.

Bug: v8:2728, v8:7828
Change-Id: Id2eea62062533eb99cd6670c42a4b1da87139008
Reviewed-on: https://chromium-review.googlesource.com/c/1382095Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58353}

src/ast/scopes.cc

@@ -1156,13 +1156,12 @@ Variable* Scope::DeclareVariableName(const AstRawString* name,
   return var;
 }
 
-void Scope::DeclareCatchVariableName(const AstRawString* name) {
+Variable* Scope::DeclareCatchVariableName(const AstRawString* name) {
   DCHECK(!already_resolved_);
-  DCHECK(GetDeclarationScope()->is_being_lazily_parsed());
   DCHECK(is_catch_scope());
   DCHECK(scope_info_.is_null());
 
-  Declare(zone(), name, VariableMode::kVar);
+  return Declare(zone(), name, VariableMode::kVar);
 }
 
 void Scope::AddUnresolved(VariableProxy* proxy) {
@@ -1238,24 +1237,29 @@ Declaration* Scope::CheckConflictingVarDeclarations() {
   return nullptr;
 }
 
-Declaration* Scope::CheckLexDeclarationsConflictingWith(
-    const ZonePtrList<const AstRawString>& names) {
+const AstRawString* Scope::FindLexVariableDeclaredIn(Scope* scope) {
   DCHECK(is_block_scope());
-  for (int i = 0; i < names.length(); ++i) {
-    Variable* var = LookupLocal(names.at(i));
+  const VariableMap& variables = scope->variables_;
+  for (ZoneHashMap::Entry* p = variables.Start(); p != nullptr;
+       p = variables.Next(p)) {
+    const AstRawString* name = static_cast<const AstRawString*>(p->key);
+    Variable* var = LookupLocal(name);
     if (var != nullptr) {
       // Conflict; find and return its declaration.
       DCHECK(IsLexicalVariableMode(var->mode()));
-      const AstRawString* name = names.at(i);
-      for (Declaration* decl : decls_) {
-        if (decl->proxy()->raw_name() == name) return decl;
-      }
-      DCHECK(false);
+      return name;
     }
   }
   return nullptr;
 }
 
+Declaration* Scope::DeclarationFor(const AstRawString* name) {
+  for (Declaration* decl : decls_) {
+    if (decl->proxy()->raw_name() == name) return decl;
+  }
+  UNREACHABLE();
+}
+
 bool DeclarationScope::AllocateVariables(ParseInfo* info) {
   // Module variables must be allocated before variable resolution
   // to ensure that UpdateNeedsHoleCheck() can detect import variables.

src/ast/scopes.h

@@ -258,7 +258,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
 
   // The return value is meaningful only if FLAG_preparser_scope_analysis is on.
   Variable* DeclareVariableName(const AstRawString* name, VariableMode mode);
-  void DeclareCatchVariableName(const AstRawString* name);
+  Variable* DeclareCatchVariableName(const AstRawString* name);
 
   // Declarations list.
   base::ThreadedList<Declaration>* declarations() { return &decls_; }
@@ -312,13 +312,14 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(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(
-      const ZonePtrList<const AstRawString>& names);
+  // Find lexical variable that has a name that was declared in |scope|. 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. Returns the first
+  // duplicate variable name if there is one, nullptr otherwise.
+  const AstRawString* FindLexVariableDeclaredIn(Scope* scope);
+
+  // Find the declaration that introduced |name|.
+  Declaration* DeclarationFor(const AstRawString* name);
 
   // ---------------------------------------------------------------------------
   // Scope-specific info.

src/parsing/parser-base.h

@@ -519,18 +519,12 @@ class ParserBase {
   struct CatchInfo {
    public:
     explicit CatchInfo(ParserBase* parser)
-        : name(parser->impl()->NullIdentifier()),
-          pattern(parser->impl()->NullExpression()),
-          scope(nullptr),
-          init_block(parser->impl()->NullStatement()),
-          inner_block(parser->impl()->NullStatement()),
-          bound_names(1, parser->zone()) {}
-    IdentifierT name;
+        : pattern(parser->impl()->NullExpression()),
+          variable(nullptr),
+          scope(nullptr) {}
     ExpressionT pattern;
+    Variable* variable;
     Scope* scope;
-    BlockT init_block;
-    BlockT inner_block;
-    ZonePtrList<const AstRawString> bound_names;
   };
 
   struct ForInfo {
@@ -3515,7 +3509,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
   parsing_result->descriptor.declaration_pos = peek_position();
   parsing_result->descriptor.initialization_pos = peek_position();
 
-  BlockT init_block = impl()->NullStatement();
+  BlockT init_block = impl()->NullBlock();
   if (var_context != kForStatement) {
     init_block = factory()->NewBlock(1, true);
   }
@@ -3611,7 +3605,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseVariableDeclarations(
               Scanner::Location(decl_pos, end_position()),
               MessageTemplate::kDeclarationMissingInitializer,
               !impl()->IsIdentifier(pattern) ? "destructuring" : "const");
-          return impl()->NullStatement();
+          return impl()->NullBlock();
         }
         // 'let x' initializes 'x' to undefined.
         if (parsing_result->descriptor.mode == VariableMode::kLet) {
@@ -4234,7 +4228,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseClassLiteral(
 template <typename Impl>
 void ParserBase<Impl>::ParseAsyncFunctionBody(Scope* scope,
                                               StatementListT* body) {
-  BlockT block = impl()->NullStatement();
+  BlockT block = impl()->NullBlock();
   {
     StatementListT statements(pointer_buffer());
     ParseStatementList(&statements, Token::RBRACE);
@@ -4736,19 +4730,19 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
   // Block ::
   //   '{' StatementList '}'
 
+  // Parse the statements and collect escaping labels.
   BlockT body = factory()->NewBlock(false, labels);
   StatementListT statements(pointer_buffer());
 
-  // Parse the statements and collect escaping labels.
-  Expect(Token::LBRACE);
-
   CheckStackOverflow();
 
   {
     BlockState block_state(zone(), &scope_);
-    scope()->set_start_position(scanner()->location().beg_pos);
+    scope()->set_start_position(peek_position());
     TargetT target(this, body);
 
+    Expect(Token::LBRACE);
+
     while (peek() != Token::RBRACE) {
       StatementT stat = ParseStatementListItem();
       if (impl()->IsNull(stat)) return body;
@@ -4757,6 +4751,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
     }
 
     Expect(Token::RBRACE);
+
     int end_pos = end_position();
     scope()->set_end_position(end_pos);
 
@@ -5255,7 +5250,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement() {
 
   SourceRange catch_range, finally_range;
 
-  BlockT catch_block = impl()->NullStatement();
+  BlockT catch_block = impl()->NullBlock();
   {
     SourceRangeScope catch_range_scope(scanner(), &catch_range);
     if (Check(Token::CATCH)) {
@@ -5274,31 +5269,44 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement() {
           // as part of destructuring the catch parameter.
           {
             BlockState catch_variable_block_state(zone(), &scope_);
-            scope()->set_start_position(scanner()->location().beg_pos);
+            scope()->set_start_position(position());
 
-            // This does not simply call ParsePrimaryExpression to avoid
-            // ExpressionFromIdentifier from being called in the first
-            // branch, which would introduce an unresolved symbol and mess
-            // with arrow function names.
             if (peek_any_identifier()) {
-              catch_info.name = ParseNonRestrictedIdentifier();
+              IdentifierT identifier = ParseNonRestrictedIdentifier();
+              RETURN_IF_PARSE_ERROR;
+              catch_info.variable = impl()->DeclareCatchVariableName(
+                  catch_info.scope, identifier);
             } else {
-              DeclarationParsingScope declaration(
-                  impl(), ExpressionScope::kVarDeclaration);
+              catch_info.variable = catch_info.scope->DeclareCatchVariableName(
+                  ast_value_factory()->dot_catch_string());
+              DeclarationParsingScope destructuring(
+                  impl(), ExpressionScope::kLexicalDeclaration);
               catch_info.pattern = ParseBindingPattern();
+              RETURN_IF_PARSE_ERROR;
+              catch_statements.Add(impl()->RewriteCatchPattern(&catch_info));
             }
 
             Expect(Token::RPAREN);
-            RETURN_IF_PARSE_ERROR;
-            impl()->RewriteCatchPattern(&catch_info);
-            if (!impl()->IsNull(catch_info.init_block)) {
-              catch_statements.Add(catch_info.init_block);
+
+            BlockT inner_block = ParseBlock(nullptr);
+            catch_statements.Add(inner_block);
+
+            // Check for `catch(e) { let e; }` and similar errors.
+            Scope* inner_scope = inner_block->scope();
+            if (inner_scope != nullptr) {
+              const AstRawString* conflict = nullptr;
+              if (impl()->IsNull(catch_info.pattern)) {
+                const AstRawString* name = catch_info.variable->raw_name();
+                if (inner_scope->LookupLocal(name)) conflict = name;
+              } else {
+                conflict = inner_scope->FindLexVariableDeclaredIn(scope());
+              }
+              if (conflict != nullptr) {
+                impl()->ReportConflictingDeclarationInCatch(conflict,
+                                                            inner_scope);
+              }
             }
 
-            catch_info.inner_block = ParseBlock(nullptr);
-            catch_statements.Add(catch_info.inner_block);
-            RETURN_IF_PARSE_ERROR;
-            impl()->ValidateCatchBlock(catch_info);
             scope()->set_end_position(end_position());
             catch_block = factory()->NewBlock(false, catch_statements);
             catch_block->set_scope(scope()->FinalizeBlockScope());
@@ -5312,7 +5320,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement() {
     }
   }
 
-  BlockT finally_block = impl()->NullStatement();
+  BlockT finally_block = impl()->NullBlock();
   DCHECK(has_error() || peek() == Token::FINALLY ||
          !impl()->IsNull(catch_block));
   {
@@ -5499,7 +5507,7 @@ ParserBase<Impl>::ParseForEachStatementWithDeclarations(
   }
 
   ExpressionT each_variable = impl()->NullExpression();
-  BlockT body_block = impl()->NullStatement();
+  BlockT body_block = impl()->NullBlock();
   {
     BlockState block_state(
         &scope_, inner_block_scope != nullptr ? inner_block_scope : scope_);
@@ -5777,7 +5785,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
     impl()->RecordIterationStatementSourceRange(loop, body_range);
 
     if (has_declarations) {
-      BlockT body_block = impl()->NullStatement();
+      BlockT body_block = impl()->NullBlock();
       impl()->DesugarBindingInForEachStatement(&for_info, &body_block,
                                                &each_variable);
       body_block->statements()->Add(body, zone());
@@ -5802,7 +5810,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
   }
 
   BlockT init_block =
-      impl()->CreateForEachStatementTDZ(impl()->NullStatement(), for_info);
+      impl()->CreateForEachStatementTDZ(impl()->NullBlock(), for_info);
 
   scope()->set_end_position(end_position());
   Scope* for_scope = scope()->FinalizeBlockScope();

src/parsing/parser.cc

@@ -1568,52 +1568,41 @@ Statement* Parser::RewriteSwitchStatement(SwitchStatement* switch_statement,
   return switch_block;
 }
 
-void Parser::RewriteCatchPattern(CatchInfo* catch_info) {
-  if (catch_info->name == nullptr) {
-    DCHECK_NOT_NULL(catch_info->pattern);
-    catch_info->name = ast_value_factory()->dot_catch_string();
-  }
-  Variable* catch_variable =
-      catch_info->scope->DeclareLocal(catch_info->name, VariableMode::kVar);
-  if (catch_info->pattern != nullptr) {
-    DeclarationDescriptor descriptor;
-    descriptor.declaration_kind = DeclarationDescriptor::NORMAL;
-    descriptor.scope = scope();
-    descriptor.mode = VariableMode::kLet;
-    descriptor.declaration_pos = catch_info->pattern->position();
-    descriptor.initialization_pos = catch_info->pattern->position();
-
-    // Initializer position for variables declared by the pattern.
-    const int initializer_position = position();
-
-    DeclarationParsingResult::Declaration decl(
-        catch_info->pattern, initializer_position,
-        factory()->NewVariableProxy(catch_variable));
-
-    catch_info->init_block = factory()->NewBlock(8, true);
-    DeclareAndInitializeVariables(catch_info->init_block, &descriptor, &decl,
-                                  &catch_info->bound_names);
-  } else {
-    catch_info->bound_names.Add(catch_info->name, zone());
-  }
+Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) {
+  DCHECK_NOT_NULL(catch_info->pattern);
+  DeclarationDescriptor descriptor;
+  descriptor.declaration_kind = DeclarationDescriptor::NORMAL;
+  descriptor.scope = scope();
+  descriptor.mode = VariableMode::kLet;
+  descriptor.declaration_pos = catch_info->pattern->position();
+  descriptor.initialization_pos = catch_info->pattern->position();
+
+  // Initializer position for variables declared by the pattern.
+  const int initializer_position = position();
+
+  DeclarationParsingResult::Declaration decl(
+      catch_info->pattern, initializer_position,
+      factory()->NewVariableProxy(catch_info->variable));
+
+  Block* init_block = factory()->NewBlock(8, true);
+  DeclareAndInitializeVariables(init_block, &descriptor, &decl, nullptr);
+  return init_block;
 }
 
-void Parser::ValidateCatchBlock(const CatchInfo& catch_info) {
-  // Check for `catch(e) { let e; }` and similar errors.
-  Scope* inner_block_scope = catch_info.inner_block->scope();
-  if (inner_block_scope != nullptr) {
-    Declaration* decl = inner_block_scope->CheckLexDeclarationsConflictingWith(
-        catch_info.bound_names);
-    if (decl != nullptr) {
-      const AstRawString* name = decl->proxy()->raw_name();
+void Parser::ReportConflictingDeclarationInCatch(const AstRawString* name,
+                                                 Scope* scope) {
+  for (Declaration* decl : *scope->declarations()) {
+    if (decl->proxy()->raw_name() == name) {
       int position = decl->proxy()->position();
       Scanner::Location location =
           position == kNoSourcePosition
               ? Scanner::Location::invalid()
-              : Scanner::Location(position, position + 1);
+              : Scanner::Location(position, position + name->length());
       ReportMessageAt(location, MessageTemplate::kVarRedeclaration, name);
+      return;
     }
   }
+  UNREACHABLE();
 }
 
 Statement* Parser::RewriteTryStatement(Block* try_block, Block* catch_block,

src/parsing/parser.h

@@ -317,8 +317,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   Expression* RewriteReturn(Expression* return_value, int pos);
   Statement* RewriteSwitchStatement(SwitchStatement* switch_statement,
                                     Scope* scope);
-  void RewriteCatchPattern(CatchInfo* catch_info);
-  void ValidateCatchBlock(const CatchInfo& catch_info);
+  Block* RewriteCatchPattern(CatchInfo* catch_info);
+  void ReportConflictingDeclarationInCatch(const AstRawString* name,
+                                           Scope* scope);
   Statement* RewriteTryStatement(Block* try_block, Block* catch_block,
                                  const SourceRange& catch_range,
                                  Block* finally_block,
@@ -830,6 +831,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     return nullptr;
   }
   V8_INLINE static std::nullptr_t NullStatement() { return nullptr; }
+  V8_INLINE static std::nullptr_t NullBlock() { return nullptr; }
   Expression* FailureExpression() { return factory()->FailureExpression(); }
 
   template <typename T>
@@ -881,6 +883,11 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     return NewUnresolved(name, start_position);
   }
 
+  V8_INLINE Variable* DeclareCatchVariableName(Scope* scope,
+                                               const AstRawString* name) {
+    return scope->DeclareCatchVariableName(name);
+  }
+
   V8_INLINE ZonePtrList<Expression>* NewExpressionList(int size) const {
     return new (zone()) ZonePtrList<Expression>(size, zone());
   }

src/parsing/preparser.cc

@@ -381,7 +381,7 @@ PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
   return kLazyParsingComplete;
 }
 
-PreParserStatement PreParser::BuildParameterInitializationBlock(
+PreParserBlock PreParser::BuildParameterInitializationBlock(
     const PreParserFormalParameters& parameters) {
   DCHECK(!parameters.is_simple);
   DCHECK(scope()->is_function_scope());
@@ -402,7 +402,7 @@ PreParserStatement PreParser::BuildParameterInitializationBlock(
     }
   }
 
-  return PreParserStatement::Default();
+  return PreParserBlock::Default();
 }
 
 bool PreParser::IdentifierEquals(const PreParserIdentifier& identifier,

src/parsing/preparser.h

@@ -524,7 +524,7 @@ class PreParserStatement {
                   PreParserStatement next, PreParserStatement body,
                   const SourceRange& body_range = {}) {}
 
- private:
+ protected:
   enum Type {
     kNullStatement,
     kEmptyStatement,
@@ -534,9 +534,32 @@ class PreParserStatement {
   };
 
   explicit PreParserStatement(Type code) : code_(code) {}
+
+ private:
   Type code_;
 };
 
+// A PreParserBlock extends statement with a place to store the scope.
+// The scope is dropped as the block is returned as a statement.
+class PreParserBlock : public PreParserStatement {
+ public:
+  void set_scope(Scope* scope) { scope_ = scope; }
+  Scope* scope() const { return scope_; }
+  static PreParserBlock Default() {
+    return PreParserBlock(PreParserStatement::kUnknownStatement);
+  }
+  static PreParserBlock Null() {
+    return PreParserBlock(PreParserStatement::kNullStatement);
+  }
+  // Dummy implementation for making block->somefunc() work in both Parser and
+  // PreParser.
+  PreParserBlock* operator->() { return this; }
+
+ private:
+  explicit PreParserBlock(PreParserStatement::Type type)
+      : PreParserStatement(type), scope_(nullptr) {}
+  Scope* scope_;
+};
 
 class PreParserFactory {
  public:
@@ -722,18 +745,18 @@ class PreParserFactory {
 
   PreParserStatement EmptyStatement() { return PreParserStatement::Default(); }
 
-  PreParserStatement NewBlock(int capacity, bool ignore_completion_value) {
-    return PreParserStatement::Default();
+  PreParserBlock NewBlock(int capacity, bool ignore_completion_value) {
+    return PreParserBlock::Default();
   }
 
-  PreParserStatement NewBlock(bool ignore_completion_value,
-                              ZonePtrList<const AstRawString>* labels) {
-    return PreParserStatement::Default();
+  PreParserBlock NewBlock(bool ignore_completion_value,
+                          ZonePtrList<const AstRawString>* labels) {
+    return PreParserBlock::Default();
   }
 
-  PreParserStatement NewBlock(bool ignore_completion_value,
-                              const PreParserScopedStatementList& list) {
-    return PreParserStatement::Default();
+  PreParserBlock NewBlock(bool ignore_completion_value,
+                          const PreParserScopedStatementList& list) {
+    return PreParserBlock::Default();
   }
 
   PreParserStatement NewDebuggerStatement(int pos) {
@@ -927,7 +950,7 @@ struct ParserTypes<PreParser> {
   typedef PreParserIdentifier Identifier;
   typedef PreParserPropertyList ClassPropertyList;
   typedef PreParserScopedStatementList StatementList;
-  typedef PreParserStatement Block;
+  typedef PreParserBlock Block;
   typedef PreParserStatement BreakableStatement;
   typedef PreParserStatement ForStatement;
   typedef PreParserStatement IterationStatement;
@@ -1131,21 +1154,20 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Default();
   }
 
-  V8_INLINE void RewriteCatchPattern(CatchInfo* catch_info) {
-    const AstRawString* catch_name = catch_info->name.string_;
-    if (catch_name == nullptr) {
-      catch_name = ast_value_factory()->dot_catch_string();
-    }
-    catch_info->scope->DeclareCatchVariableName(catch_name);
-
+  V8_INLINE PreParserBlock RewriteCatchPattern(CatchInfo* catch_info) {
     if (catch_info->pattern.variables_ != nullptr) {
       for (auto variable : *catch_info->pattern.variables_) {
         scope()->DeclareVariableName(variable->raw_name(), VariableMode::kLet);
       }
     }
+    return PreParserBlock::Default();
+  }
+
+  V8_INLINE void ReportConflictingDeclarationInCatch(const AstRawString* name,
+                                                     Scope* scope) {
+    ReportUnidentifiableError();
   }
 
-  V8_INLINE void ValidateCatchBlock(const CatchInfo& catch_info) {}
   V8_INLINE PreParserStatement RewriteTryStatement(
       PreParserStatement try_block, PreParserStatement catch_block,
       const SourceRange& catch_range, PreParserStatement finally_block,
@@ -1447,8 +1469,8 @@ class PreParser : public ParserBase<PreParser> {
     return stmt;
   }
 
-  V8_INLINE PreParserStatement RewriteForVarInLegacy(const ForInfo& for_info) {
-    return PreParserStatement::Null();
+  V8_INLINE PreParserBlock RewriteForVarInLegacy(const ForInfo& for_info) {
+    return PreParserBlock::Null();
   }
 
   V8_INLINE void DesugarBindingInForEachStatement(
@@ -1468,13 +1490,13 @@ class PreParser : public ParserBase<PreParser> {
         collect_names ? &for_info->bound_names : nullptr);
   }
 
-  V8_INLINE PreParserStatement CreateForEachStatementTDZ(
-      PreParserStatement init_block, const ForInfo& for_info) {
+  V8_INLINE PreParserBlock CreateForEachStatementTDZ(PreParserBlock init_block,
+                                                     const ForInfo& for_info) {
     if (IsLexicalVariableMode(for_info.parsing_result.descriptor.mode)) {
       for (auto name : for_info.bound_names) {
         scope()->DeclareVariableName(name, VariableMode::kLet);
       }
-      return PreParserStatement::Default();
+      return PreParserBlock::Default();
     }
     return init_block;
   }
@@ -1491,12 +1513,12 @@ class PreParser : public ParserBase<PreParser> {
     return loop;
   }
 
-  PreParserStatement BuildParameterInitializationBlock(
+  PreParserBlock BuildParameterInitializationBlock(
       const PreParserFormalParameters& parameters);
 
-  V8_INLINE PreParserStatement
+  V8_INLINE PreParserBlock
   BuildRejectPromiseOnException(PreParserStatement init_block) {
-    return PreParserStatement::Default();
+    return PreParserBlock::Default();
   }
 
   V8_INLINE void InsertSloppyBlockFunctionVarBindings(DeclarationScope* scope) {
@@ -1562,6 +1584,7 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE static PreParserStatement NullStatement() {
     return PreParserStatement::Null();
   }
+  V8_INLINE static PreParserBlock NullBlock() { return PreParserBlock::Null(); }
 
   template <typename T>
   V8_INLINE static bool IsNull(T subject) {
@@ -1631,6 +1654,11 @@ class PreParser : public ParserBase<PreParser> {
       const PreParserIdentifier& name, int start_position,
       InferName infer = InferName::kYes);
 
+  V8_INLINE Variable* DeclareCatchVariableName(
+      Scope* scope, const PreParserIdentifier& identifier) {
+    return scope->DeclareCatchVariableName(identifier.string_);
+  }
+
   V8_INLINE PreParserPropertyList NewClassPropertyList(int size) const {
     return PreParserPropertyList();
   }

test/message/fail/try-catch-lexical-conflict-preparser.js

+// Copyright 2018 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 f() {
+  try { }
+  catch ({x}) {
+    let x
+  }
+}

test/message/fail/try-catch-lexical-conflict-preparser.out

+*%(basename)s:8: SyntaxError: Identifier 'x' has already been declared
+    let x
+        ^
+SyntaxError: Identifier 'x' has already been declared

test/mjsunit/try-catch-default-destructuring.js

+// Copyright 2018 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 f1() {
+  let y = 200;
+  try {
+    throw {}
+  } catch ({x=()=>y, y=300}) {
+    return x()
+  }
+}
+assertEquals(300, f1());
+
+function f2() {
+  let y = 200;
+  try {
+    throw {}
+  } catch ({x=()=>y}) {
+    let y = 300;
+    return x()
+  }
+}
+assertEquals(200, f2());

test/test262/test262.status

@@ -406,9 +406,6 @@
   'language/literals/regexp/u-unicode-esc-non-hex': [FAIL_PHASE_ONLY],
   'language/literals/regexp/unicode-escape-nls-err': [FAIL_PHASE_ONLY],
 
-  # https://bugs.chromium.org/p/v8/issues/detail?id=7828
-  'language/statements/try/early-catch-function': [FAIL],
-
   # https://bugs.chromium.org/p/v8/issues/detail?id=7829
   'language/block-scope/syntax/redeclaration/function-declaration-attempt-to-redeclare-with-var-declaration-nested-in-function': [FAIL],