[language] Implement optional catch binding proposal

This allows the syntax `try {} catch {}` (with no binding after the `catch`). See https://github.com/michaelficarra/optional-catch-binding-proposal/ Currently behind --harmony-optional-catch-binding. As part of the implementation, this allows TryCatchStatements to not have an associated catch scope; various paths which assumed they would have been updated to handle this case. Cq-Include-Trybots: master.tryserver.v8:v8_linux_noi18n_rel_ng Change-Id: Ic525b45199eef025eb05da562e10fbd4f3d7465f Reviewed-on: https://chromium-review.googlesource.com/571453 Reviewed-by: Marja Hölttä <[email protected]> Reviewed-by: Adam Klein <[email protected]> Reviewed-by: Sathya Gunasekaran <[email protected]> Reviewed-by: Ross McIlroy <[email protected]> Commit-Queue: Kevin Gibbons <[email protected]> Cr-Commit-Position: refs/heads/master@{#48300}
paul99 · Oct 5, 2017 · d0651bd · d0651bd
1 parent f83d0e0
commit d0651bd
Show file tree

Hide file tree

Showing 18 changed files with 429 additions and 354 deletions.
diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc
@@ -924,8 +924,10 @@ void AstPrinter::VisitTryCatchStatement(TryCatchStatement* node) {
       UNREACHABLE();
   }
   Print(" %s\n", prediction);
-  PrintLiteralWithModeIndented("CATCHVAR", node->scope()->catch_variable(),
-                               node->scope()->catch_variable()->name());
+  if (node->scope()) {
+    PrintLiteralWithModeIndented("CATCHVAR", node->scope()->catch_variable(),
+                                 node->scope()->catch_variable()->name());
+  }
   PrintIndentedVisit("CATCH", node->catch_block());
 }
 

diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
@@ -4224,6 +4224,7 @@ EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_dynamic_import)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_template_escapes)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_restrict_constructor_return)
 EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_strict_legacy_accessor_builtins)
+EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_optional_catch_binding)
 
 void InstallPublicSymbol(Factory* factory, Handle<Context> native_context,
                          const char* name, Handle<Symbol> value) {

diff --git a/src/flag-definitions.h b/src/flag-definitions.h
@@ -194,7 +194,8 @@ DEFINE_IMPLICATION(es_staging, harmony)
   V(harmony_function_sent, "harmony function.sent")                   \
   V(harmony_do_expressions, "harmony do-expressions")                 \
   V(harmony_class_fields, "harmony public fields in class literals")  \
-  V(harmony_bigint, "harmony arbitrary precision integers")
+  V(harmony_bigint, "harmony arbitrary precision integers")           \
+  V(harmony_optional_catch_binding, "allow omitting binding in catch blocks")
 
 // Features that are complete (but still behind --harmony/es-staging flag).
 #define HARMONY_STAGED_BASE(V)                                          \

diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
@@ -1615,9 +1615,11 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
   }
   try_control_builder.EndTry();
 
-  // Create a catch scope that binds the exception.
-  BuildNewLocalCatchContext(stmt->scope());
-  builder()->StoreAccumulatorInRegister(context);
+  if (stmt->scope()) {
+    // Create a catch scope that binds the exception.
+    BuildNewLocalCatchContext(stmt->scope());
+    builder()->StoreAccumulatorInRegister(context);
+  }
 
   // If requested, clear message object as we enter the catch block.
   if (stmt->ShouldClearPendingException(outer_catch_prediction)) {
@@ -1629,7 +1631,11 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) {
 
   // Evaluate the catch-block.
   BuildIncrementBlockCoverageCounterIfEnabled(stmt, SourceRangeKind::kCatch);
-  VisitInScope(stmt->catch_block(), stmt->scope());
+  if (stmt->scope()) {
+    VisitInScope(stmt->catch_block(), stmt->scope());
+  } else {
+    VisitBlock(stmt->catch_block());
+  }
   try_control_builder.EndCatch();
   BuildIncrementBlockCoverageCounterIfEnabled(stmt,
                                               SourceRangeKind::kContinuation);

diff --git a/src/parsing/expression-scope-reparenter.cc b/src/parsing/expression-scope-reparenter.cc
@@ -73,15 +73,19 @@ void Reparenter::VisitVariableProxy(VariableProxy* proxy) {
 }
 
 void Reparenter::VisitBlock(Block* stmt) {
-  if (stmt->scope() != nullptr)
+  if (stmt->scope())
     stmt->scope()->ReplaceOuterScope(scope_);
   else
     VisitStatements(stmt->statements());
 }
 
 void Reparenter::VisitTryCatchStatement(TryCatchStatement* stmt) {
   Visit(stmt->try_block());
-  stmt->scope()->ReplaceOuterScope(scope_);
+  if (stmt->scope()) {
+    stmt->scope()->ReplaceOuterScope(scope_);
+  } else {
+    Visit(stmt->catch_block());
+  }
 }
 
 void Reparenter::VisitWithStatement(WithStatement* stmt) {

diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
@@ -278,7 +278,8 @@ class ParserBase {
         allow_harmony_object_rest_spread_(false),
         allow_harmony_dynamic_import_(false),
         allow_harmony_async_iteration_(false),
-        allow_harmony_template_escapes_(false) {}
+        allow_harmony_template_escapes_(false),
+        allow_harmony_optional_catch_binding_(false) {}
 
 #define ALLOW_ACCESSORS(name)                           \
   bool allow_##name() const { return allow_##name##_; } \
@@ -293,6 +294,7 @@ class ParserBase {
   ALLOW_ACCESSORS(harmony_dynamic_import);
   ALLOW_ACCESSORS(harmony_async_iteration);
   ALLOW_ACCESSORS(harmony_template_escapes);
+  ALLOW_ACCESSORS(harmony_optional_catch_binding);
 
 #undef ALLOW_ACCESSORS
 
@@ -1497,6 +1499,7 @@ class ParserBase {
   bool allow_harmony_dynamic_import_;
   bool allow_harmony_async_iteration_;
   bool allow_harmony_template_escapes_;
+  bool allow_harmony_optional_catch_binding_;
 
   friend class DiscardableZoneScope;
 };
@@ -5467,50 +5470,60 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseTryStatement(
   {
     SourceRangeScope catch_range_scope(scanner(), &catch_range);
     if (Check(Token::CATCH)) {
-      Expect(Token::LPAREN, CHECK_OK);
-      catch_info.scope = NewScope(CATCH_SCOPE);
-      catch_info.scope->set_start_position(scanner()->location().beg_pos);
-
-      {
-        BlockState catch_block_state(&scope_, catch_info.scope);
+      bool has_binding;
+      if (allow_harmony_optional_catch_binding()) {
+        has_binding = Check(Token::LPAREN);
+      } else {
+        has_binding = true;
+        Expect(Token::LPAREN, CHECK_OK);
+      }
 
-        catch_block = factory()->NewBlock(16, false);
+      if (has_binding) {
+        catch_info.scope = NewScope(CATCH_SCOPE);
+        catch_info.scope->set_start_position(scanner()->location().beg_pos);
 
-        // Create a block scope to hold any lexical declarations created
-        // as part of destructuring the catch parameter.
         {
-          BlockState catch_variable_block_state(zone(), &scope_);
-          scope()->set_start_position(scanner()->location().beg_pos);
-          typename Types::Target target(this, catch_block);
-
-          // 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 =
-                ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
-          } else {
-            ExpressionClassifier pattern_classifier(this);
-            catch_info.pattern = ParsePrimaryExpression(CHECK_OK);
-            ValidateBindingPattern(CHECK_OK);
-          }
+          BlockState catch_block_state(&scope_, catch_info.scope);
+
+          catch_block = factory()->NewBlock(16, false);
+
+          // Create a block scope to hold any lexical declarations created
+          // as part of destructuring the catch parameter.
+          {
+            BlockState catch_variable_block_state(zone(), &scope_);
+            scope()->set_start_position(scanner()->location().beg_pos);
+
+            // 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 =
+                  ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
+            } else {
+              ExpressionClassifier pattern_classifier(this);
+              catch_info.pattern = ParsePrimaryExpression(CHECK_OK);
+              ValidateBindingPattern(CHECK_OK);
+            }
 
-          Expect(Token::RPAREN, CHECK_OK);
-          impl()->RewriteCatchPattern(&catch_info, CHECK_OK);
-          if (!impl()->IsNull(catch_info.init_block)) {
-            catch_block->statements()->Add(catch_info.init_block, zone());
-          }
+            Expect(Token::RPAREN, CHECK_OK);
+            impl()->RewriteCatchPattern(&catch_info, CHECK_OK);
+            if (!impl()->IsNull(catch_info.init_block)) {
+              catch_block->statements()->Add(catch_info.init_block, zone());
+            }
 
-          catch_info.inner_block = ParseBlock(nullptr, CHECK_OK);
-          catch_block->statements()->Add(catch_info.inner_block, zone());
-          impl()->ValidateCatchBlock(catch_info, CHECK_OK);
-          scope()->set_end_position(scanner()->location().end_pos);
-          catch_block->set_scope(scope()->FinalizeBlockScope());
+            catch_info.inner_block = ParseBlock(nullptr, CHECK_OK);
+            catch_block->statements()->Add(catch_info.inner_block, zone());
+            impl()->ValidateCatchBlock(catch_info, CHECK_OK);
+            scope()->set_end_position(scanner()->location().end_pos);
+            catch_block->set_scope(scope()->FinalizeBlockScope());
+          }
         }
-      }
 
-      catch_info.scope->set_end_position(scanner()->location().end_pos);
+        catch_info.scope->set_end_position(scanner()->location().end_pos);
+      } else {
+        catch_block = ParseBlock(nullptr, CHECK_OK);
+      }
     }
   }
 

diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
@@ -506,6 +506,7 @@ Parser::Parser(ParseInfo* info)
   set_allow_harmony_dynamic_import(FLAG_harmony_dynamic_import);
   set_allow_harmony_async_iteration(FLAG_harmony_async_iteration);
   set_allow_harmony_template_escapes(FLAG_harmony_template_escapes);
+  set_allow_harmony_optional_catch_binding(FLAG_harmony_optional_catch_binding);
   for (int feature = 0; feature < v8::Isolate::kUseCounterFeatureCount;
        ++feature) {
     use_counts_[feature] = 0;
@@ -1661,7 +1662,6 @@ Statement* Parser::RewriteTryStatement(Block* try_block, Block* catch_block,
 
   if (catch_block != nullptr && finally_block != nullptr) {
     // If we have both, create an inner try/catch.
-    DCHECK_NOT_NULL(catch_info.scope);
     TryCatchStatement* statement;
     statement = factory()->NewTryCatchStatement(try_block, catch_info.scope,
                                                 catch_block, kNoSourcePosition);
@@ -1674,7 +1674,6 @@ Statement* Parser::RewriteTryStatement(Block* try_block, Block* catch_block,
 
   if (catch_block != nullptr) {
     DCHECK_NULL(finally_block);
-    DCHECK_NOT_NULL(catch_info.scope);
     TryCatchStatement* stmt = factory()->NewTryCatchStatement(
         try_block, catch_info.scope, catch_block, pos);
     RecordTryCatchStatementSourceRange(stmt, catch_range);
@@ -4294,9 +4293,8 @@ void Parser::BuildIteratorCloseForCompletion(ZoneList<Statement*>* statements,
                                  zone());
 
     Block* catch_block = factory()->NewBlock(0, false);
-    Scope* catch_scope = NewHiddenCatchScope();
-    try_call_return = factory()->NewTryCatchStatement(try_block, catch_scope,
-                                                      catch_block, nopos);
+    try_call_return =
+        factory()->NewTryCatchStatement(try_block, nullptr, catch_block, nopos);
   }
 
   // let output = %_Call(iteratorReturn, iterator);

diff --git a/src/parsing/parser.h b/src/parsing/parser.h
@@ -298,6 +298,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       SET_ALLOW(harmony_async_iteration);
       SET_ALLOW(harmony_template_escapes);
       SET_ALLOW(harmony_restrictive_generators);
+      SET_ALLOW(harmony_optional_catch_binding);
 #undef SET_ALLOW
     }
     return reusable_preparser_;

diff --git a/test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden b/test/cctest/interpreter/bytecode_expectations/AsyncGenerators.golden
@@ -292,7 +292,7 @@ snippet: "
 "
 frame size: 22
 parameter count: 1
-bytecode array length: 571
+bytecode array length: 557
 bytecodes: [
                 B(Ldar), R(2),
                 B(JumpIfUndefined), U8(18),
@@ -326,7 +326,7 @@ bytecodes: [
                 B(LdaZero),
                 B(Star), R(11),
                 B(Mov), R(15), R(12),
-                B(JumpConstant), U8(21),
+                B(JumpConstant), U8(20),
                 B(LdaZero),
                 B(Star), R(6),
                 B(Mov), R(context), R(17),
@@ -415,15 +415,15 @@ bytecodes: [
                 B(Star), R(17),
                 B(LdaZero),
                 B(TestEqualStrict), R(6), U8(14),
-                B(JumpIfTrue), U8(104),
+                B(JumpIfTrue), U8(90),
                 B(LdaNamedProperty), R(4), U8(15), U8(15),
                 B(Star), R(8),
                 B(TestUndetectable),
                 B(JumpIfFalse), U8(4),
-                B(Jump), U8(93),
+                B(Jump), U8(79),
                 B(LdaSmi), I8(1),
                 B(TestEqualStrict), R(6), U8(18),
-                B(JumpIfFalse), U8(61),
+                B(JumpIfFalse), U8(47),
                 B(Ldar), R(8),
                 B(TestTypeOf), U8(5),
                 B(JumpIfFalse), U8(4),
@@ -438,16 +438,10 @@ bytecodes: [
                 B(Mov), R(8), R(19),
                 B(Mov), R(4), R(20),
                 B(InvokeIntrinsic), U8(Runtime::k_Call), R(19), U8(2),
-                B(Jump), U8(20),
-                B(Star), R(19),
-                B(Ldar), R(closure),
-                B(CreateCatchContext), R(19), U8(13), U8(17),
-                B(Star), R(18),
+                B(Jump), U8(6),
                 B(LdaTheHole),
                 B(SetPendingMessage),
                 B(Ldar), R(18),
-                B(PushContext), R(19),
-                B(PopContext), R(19),
                 B(Jump), U8(27),
                 B(Mov), R(8), R(18),
                 B(Mov), R(4), R(19),
@@ -460,7 +454,7 @@ bytecodes: [
                 B(Ldar), R(17),
                 B(SetPendingMessage),
                 B(Ldar), R(15),
-                B(SwitchOnSmiNoFeedback), U8(18), U8(2), I8(0),
+                B(SwitchOnSmiNoFeedback), U8(17), U8(2), I8(0),
                 B(Jump), U8(13),
                 B(LdaZero),
                 B(Star), R(11),
@@ -493,7 +487,7 @@ bytecodes: [
                 B(Jump), U8(39),
                 B(Star), R(15),
                 B(Ldar), R(closure),
-                B(CreateCatchContext), R(15), U8(13), U8(20),
+                B(CreateCatchContext), R(15), U8(13), U8(19),
                 B(Star), R(14),
                 B(LdaTheHole),
                 B(SetPendingMessage),
@@ -522,7 +516,7 @@ bytecodes: [
                 B(Ldar), R(13),
                 B(SetPendingMessage),
                 B(Ldar), R(11),
-                B(SwitchOnSmiNoFeedback), U8(22), U8(3), I8(0),
+                B(SwitchOnSmiNoFeedback), U8(21), U8(3), I8(0),
                 B(Jump), U8(22),
                 B(LdaTrue),
                 B(Star), R(16),
@@ -540,7 +534,7 @@ bytecodes: [
 constant pool: [
   Smi [37],
   Smi [104],
-  Smi [427],
+  Smi [413],
   Smi [15],
   Smi [7],
   TUPLE2_TYPE,
@@ -555,18 +549,17 @@ constant pool: [
   FIXED_ARRAY_TYPE,
   ONE_BYTE_INTERNALIZED_STRING_TYPE ["return"],
   ONE_BYTE_INTERNALIZED_STRING_TYPE [""],
-  FIXED_ARRAY_TYPE,
   Smi [6],
   Smi [14],
   FIXED_ARRAY_TYPE,
-  Smi [448],
+  Smi [434],
   Smi [6],
   Smi [20],
   Smi [23],
 ]
 handlers: [
-  [40, 516, 524],
-  [43, 477, 479],
+  [40, 502, 510],
+  [43, 463, 465],
   [90, 277, 285],
   [93, 237, 239],
   [346, 356, 358],