Implement new catch syntax in vm compiler

runtime/vm/parser.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/parser.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
 #include "vm/compiler_stats.h"
 #include "vm/dart_api_impl.h"
 #include "vm/dart_entry.h"
 #include "vm/flags.h"
 #include "vm/growable_array.h"
 #include "vm/longjump.h"
 #include "vm/native_entry.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
 #include "vm/resolver.h"
 #include "vm/scopes.h"
 #include "vm/symbols.h"
 
 namespace dart {
 
 DEFINE_FLAG(bool, enable_asserts, false, "Enable assert statements.");
 DEFINE_FLAG(bool, enable_type_checks, false, "Enable type checks.");
 DEFINE_FLAG(bool, trace_parser, false, "Trace parser operations.");
 DEFINE_FLAG(bool, warning_as_error, false, "Treat warnings as errors.");
 DEFINE_FLAG(bool, silent_warnings, false, "Silence warnings.");
+DEFINE_FLAG(bool, warn_legacy_catch, false, "Warning on legacy catch syntax");
 
 static void CheckedModeHandler(bool value) {
   FLAG_enable_asserts = value;
   FLAG_enable_type_checks = value;
 }
 
 DEFINE_FLAG_HANDLER(CheckedModeHandler,
                     enable_checked_mode,
                     "Enabled checked mode.");
 
@@ skipping 5314 matching lines @@
   condition = InsertClosureCallNodes(condition);
   condition = new UnaryOpNode(condition_pos, Token::kNOT, condition);
   AstNode* assert_throw = MakeAssertCall(condition_pos, condition_end);
   return new IfNode(condition_pos,
                     condition,
                     NodeAsSequenceNode(condition_pos, assert_throw, NULL),
                     NULL);
 }
 
 
+// TODO(hausner): This structure can be simplified once the old catch
+// syntax is removed. All catch parameters in the new syntax are final.
 struct CatchParamDesc {
   CatchParamDesc()
       : token_pos(0), type(NULL), var(NULL), is_final(false) { }
   intptr_t token_pos;
   const AbstractType* type;
   const String* var;
   bool is_final;
 };
 
 
@@ skipping 179 matching lines @@
 
   // Now parse the 'catch' blocks if any and merge all of them into
   // an if-then sequence of the different types specified using the 'is'
   // operator.
   bool catch_seen = false;
   bool generic_catch_seen = false;
   SequenceNode* catch_handler_list = NULL;
   const intptr_t handler_pos = TokenPos();
   OpenBlock();  // Start the catch block sequence.
   current_block_->scope->AddLabel(end_catch_label);
-  while (CurrentToken() == Token::kCATCH) {
-    catch_seen = true;
+  while ((CurrentToken() == Token::kCATCH) || IsLiteral("on")) {
     const intptr_t catch_pos = TokenPos();
-    ConsumeToken();  // Consume the 'catch'.
-    ExpectToken(Token::kLPAREN);
     CatchParamDesc exception_param;
     CatchParamDesc stack_trace_param;
-    ParseCatchParameter(&exception_param);
-    if (CurrentToken() == Token::kCOMMA) {
-      ConsumeToken();
-      ParseCatchParameter(&stack_trace_param);
+    catch_seen = true;
+    if (CurrentToken() == Token::kCATCH) {
+      ConsumeToken();  // Consume the 'catch'.
+      ExpectToken(Token::kLPAREN);
+      if (IsIdentifier() &&
+          ((LookaheadToken(1) == Token::kCOMMA) ||
+          (LookaheadToken(1) == Token::kRPAREN))) {
+        // New catch syntax for untyped exception variable:
+        // catch(e) or catch (e,s).
+        exception_param.is_final = true;
+        exception_param.type =
+            &AbstractType::ZoneHandle(Type::DynamicType());
+        exception_param.token_pos = TokenPos();
+        exception_param.var = ExpectIdentifier("identifier expected");
+        if (CurrentToken() == Token::kCOMMA) {
+          ConsumeToken();
+          stack_trace_param.is_final = true;
+          // TODO(hausner): Make imlicit type be StackTrace, not Dynamic.
+          stack_trace_param.type =
+              &AbstractType::ZoneHandle(Type::DynamicType());
+          stack_trace_param.token_pos = TokenPos();
+          stack_trace_param.var = ExpectIdentifier("identifier expected");
+        }
+      } else {
+        // TODO(hausner): Remove legacy syntax support.
+        if (FLAG_warn_legacy_catch) {
+          Warning("legacy catch syntax");
+        }
+        ParseCatchParameter(&exception_param);
+        if (CurrentToken() == Token::kCOMMA) {
+          ConsumeToken();
+          ParseCatchParameter(&stack_trace_param);
+        }
+      }
+    } else {
+      // on T catch(e) { ...
+      ConsumeToken();  // on
+      exception_param.is_final = true;
+      exception_param.type = &AbstractType::ZoneHandle(
+          ParseType(ClassFinalizer::kCanonicalizeWellFormed));
+      ExpectToken(Token::kCATCH);
+      ExpectToken(Token::kLPAREN);
+      exception_param.token_pos = TokenPos();
+      exception_param.var = ExpectIdentifier("identifier expected");
+      if (CurrentToken() == Token::kCOMMA) {
+        ConsumeToken();
+        stack_trace_param.is_final = true;
+        // TODO(hausner): Make imlicit type be StackTrace, not Dynamic.
+        stack_trace_param.type =
+        &AbstractType::ZoneHandle(Type::DynamicType());
+        stack_trace_param.token_pos = TokenPos();
+        stack_trace_param.var = ExpectIdentifier("identifier expected");
+      }

[siva, 2012/08/09 19:59:29]

The processing of the stacktrace part seems identical in both cases, here and
the code above. You could possibly abstract that out into a helper function.

[hausner, 2012/08/09 20:14:49]

True, but when I'll eliminate the code for the old syntax, I will restructure
this so that the catch(e,t) part is handled only once. In the meantime, I think
a bit of code duplication does not do any harm.

     }
     ExpectToken(Token::kRPAREN);
 
     // If a generic "catch all" statement has already been seen then all
     // subsequent catch statements are dead. We issue an error for now,
     // it might make sense to turn this into a warning.
     if (generic_catch_seen) {
       ErrorMsg("a generic 'catch all' statement already exists for this "
                "try block. All subsequent catch statements are dead code");
     }
@@ skipping 3379 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart