Create implicit super constructor call if necessary

runtime/vm/parser.cc

Index: runtime/vm/parser.cc
===================================================================
--- runtime/vm/parser.cc	(revision 409)
+++ runtime/vm/parser.cc	(working copy)
@@ -1199,6 +1199,38 @@
 }
 
 
+void Parser::GenerateSuperInitializerCall(const Class& cls,
+                                          LocalVariable* receiver) {
+  const intptr_t supercall_pos = token_index_;
+  const Class& super_class = Class::Handle(cls.SuperClass());
+  // Omit the implicit super() if there is no super class (i.e.
+  // we're not compiling class Object), or if the super class is an
+  // artificially generated "wrapper class" that has no constructor.
+  if (super_class.IsNull() || (super_class.num_native_fields() > 0)) {
+    return;
+  }
+  String& ctor_name = String::Handle(super_class.Name());
+  String& ctor_suffix = String::Handle(String::NewSymbol("."));
+  ctor_name = String::Concat(ctor_name, ctor_suffix);
+  ArgumentListNode* arguments = new ArgumentListNode(supercall_pos);
+  // implicit 'this' parameter is the only argument.
+  AstNode* implicit_argument = new LoadLocalNode(supercall_pos, *receiver);
+  arguments->Add(implicit_argument);
+  const Function& super_ctor = Function::ZoneHandle(
+      super_class.LookupConstructor(ctor_name));
+  if (super_ctor.IsNull() ||
+      !super_ctor.AreValidArguments(arguments->length(),
+                                    arguments->names())) {
+    ErrorMsg(supercall_pos,
+             "unresolved implicit call to super constructor '%s()'",
+             String::Handle(super_class.Name()).ToCString());
+  }
+  CheckFunctionIsCallable(supercall_pos, super_ctor);
+  current_block_->statements->Add(
+      new StaticCallNode(supercall_pos, super_ctor, arguments));
+}
+
+
 AstNode* Parser::ParseSuperInitializer(const Class& cls,
                                        LocalVariable* receiver) {
   TRACE_PARSER("ParseSuperInitializer");
@@ -1328,27 +1360,53 @@
 }
 
 
-void Parser::ParseInitializers(const Class& cls, LocalVariable* receiver) {
+void Parser::ParseInitializers(const Class& cls) {
   TRACE_PARSER("ParseInitializers");
-  AstNode* init_statement = NULL;
-  AstNode* super_init_statement = NULL;
-  // TODO(4995181): Allow super initializer to appear in any position
-  // of the initializer list.
-  if (CurrentToken() == Token::kSUPER) {
-    super_init_statement = ParseSuperInitializer(cls, receiver);
-  } else {
-    init_statement = ParseInitializer(cls, receiver);
-    current_block_->statements->Add(init_statement);
-  }
-  while (CurrentToken() == Token::kCOMMA) {
+  LocalVariable* receiver = current_block_->scope->VariableAt(0);
+  bool super_init_seen = false;
+  if (CurrentToken() == Token::kCOLON) {
     ConsumeToken();
-    init_statement = ParseInitializer(cls, receiver);
-    current_block_->statements->Add(init_statement);
+    if ((CurrentToken() == Token::kTHIS) &&
+        ((LookaheadToken(1) == Token::kLPAREN) ||
+        ((LookaheadToken(1) == Token::kPERIOD) &&
+            (LookaheadToken(3) == Token::kLPAREN)))) {
+      // Either we see this(...) or this.xxx(...) which is a
+      // redirected constructor. We don't need to check whether
+      // const fields are initialized. The other constructor will
+      // guarantee that.
+      ParseConstructorRedirection(cls, receiver);
+      return;
+    }
+
+    for (;;) {

[regis, 2011/10/13 19:32:29]

If you do not consume the colon above (and increment the look ahead count), you
can change this for loop to a more readable form:
do {
  ConsumeToken();
  ...
} while (CurrentToken() == Token::kCOMMA);

But it's fine if you leave it as is.
  

[hausner, 2011/10/13 20:24:09]

Nice idea. Done.

+      AstNode* init_statement = NULL;
+      if (CurrentToken() == Token::kSUPER) {
+        if (super_init_seen) {
+          ErrorMsg("Duplicate call to super constructor");
+        }
+        init_statement = ParseSuperInitializer(cls, receiver);
+        super_init_seen = true;
+      } else {
+        init_statement = ParseInitializer(cls, receiver);
+      }
+      current_block_->statements->Add(init_statement);
+      if (CurrentToken() != Token::kCOMMA) {
+        break;
+      }
+      ConsumeToken();
+    }
   }
-  // The call to super constructor is to be done after all initializers.
-  if (super_init_statement != NULL) {
-    current_block_->statements->Add(super_init_statement);
+
+  // Generate implicit super() if we haven't seen an explicit super call
+  // or constructor redirection.
+  // Omit the implicit super() if there is no super class (i.e.
+  // we're not compiling class Object), or if the super class is an
+  // artificially generated "wrapper class" that has no constructor.
+  if (!super_init_seen) {
+    GenerateSuperInitializerCall(cls, receiver);
   }
+
+  CheckConstFieldsInitialized(cls);
 }
 
 
@@ -1422,27 +1480,7 @@
     current_block_->statements->Add(field_init);
   }
 
-  // Super call to constructor of super class.
-  const Class& super_class = Class::Handle(cls.SuperClass());
-  ASSERT(!super_class.IsNull());
-  String& ctor_name = String::Handle(super_class.Name());
-  String& ctor_suffix = String::Handle(String::NewSymbol("."));
-  ctor_name = String::Concat(ctor_name, ctor_suffix);
-  ctor_name = String::NewSymbol(ctor_name);
-  ArgumentListNode* arguments = new ArgumentListNode(ctor_pos);
-  AstNode* implicit_argument = new LoadLocalNode(ctor_pos, *receiver);
-  arguments->Add(implicit_argument);
-  const Function& super_ctor = Function::ZoneHandle(
-      super_class.LookupConstructor(ctor_name));
-  if (super_ctor.IsNull() ||
-      !super_ctor.AreValidArgumentCounts(arguments->length(), 0)) {
-    ErrorMsg(ctor_pos,
-             "super class constructor '%s' not found",
-             ctor_name.ToCString());
-  }
-  current_block_->statements->Add(
-      new StaticCallNode(ctor_pos, super_ctor, arguments));
-
+  GenerateSuperInitializerCall(cls, receiver);
   CheckConstFieldsInitialized(cls);
 
   // Empty constructor body.
@@ -1461,6 +1499,9 @@
     return MakeImplicitConstructor(func);
   }
 
+  const Class& cls = Class::Handle(func.owner());
+  ASSERT(!cls.IsNull());
+
   // Build local scope for function.
   OpenFunctionBlock(func);
 
@@ -1502,7 +1543,6 @@
 
   GrowableArray<FieldInitExpression> initializers;
   if (func.IsConstructor()) {
-    Class& cls = Class::Handle(func.owner());
     ParseInitializedInstanceFields(cls, &initializers);
   }
 
@@ -1531,7 +1571,6 @@
   // if the function is not a constructor
   if (params.has_field_initializer) {
     LocalVariable* receiver = current_block_->scope->VariableAt(0);
-    Class& cls = Class::ZoneHandle(func.owner());
     for (int i = 0; i < params.parameters->length(); i++) {
       ParamDesc& param = (*params.parameters)[i];
       if (param.is_field_initializer) {
@@ -1562,29 +1601,7 @@
   }
 
   if (func.IsConstructor()) {
-    Class& cls = Class::ZoneHandle(func.owner());
-    bool initialized_check_needed = true;
-    if (CurrentToken() == Token::kCOLON) {
-      ConsumeToken();
-      LocalVariable* receiver = current_block_->scope->VariableAt(0);
-      ASSERT(receiver != NULL);
-      if ((CurrentToken() == Token::kTHIS) &&
-          ((LookaheadToken(1) == Token::kLPAREN) ||
-          ((LookaheadToken(1) == Token::kPERIOD) &&
-              (LookaheadToken(3) == Token::kLPAREN)))) {
-        // Either we see this(...) or this.xxx(...) which is a
-        // redirected constructor. We don't need to check whether
-        // const fields are initialized. The other constructor will
-        // guarantee that.
-        initialized_check_needed = false;
-        ParseConstructorRedirection(cls, receiver);
-      } else {
-        ParseInitializers(cls, receiver);
-      }
-    }
-    if (initialized_check_needed) {
-      CheckConstFieldsInitialized(cls);
-    }
+    ParseInitializers(cls);
   }
 
   if (current_block_->scope->function_level() > 0) {
@@ -1652,34 +1669,30 @@
 
 
 void Parser::SkipInitializers() {
-  if (CurrentToken() == Token::kSUPER) {
-    ConsumeToken();
-    if (CurrentToken() == Token::kPERIOD) {
+  for (;;) {

[regis, 2011/10/13 19:32:29]

Same comment here about a do loop instead of a for(;;).
You could delay the consume on line 1818 and start here with
ASSERT(CurrentToken() == Token::kCOLON);

[hausner, 2011/10/13 20:24:09]

And done.

+    if (CurrentToken() == Token::kSUPER) {
       ConsumeToken();
+      if (CurrentToken() == Token::kPERIOD) {
+        ConsumeToken();
+        ExpectIdentifier("identifier expected");
+      }
+      if (CurrentToken() != Token::kLPAREN) {
+        ErrorMsg("'(' expected");
+      }
+      SkipToMatchingParenthesis();
+    } else {
+      SkipIf(Token::kTHIS);
+      SkipIf(Token::kPERIOD);
       ExpectIdentifier("identifier expected");
+      ExpectToken(Token::kASSIGN);
+      SetAllowFunctionLiterals(false);
+      SkipExpr();
+      SetAllowFunctionLiterals(true);
     }
-    if (CurrentToken() != Token::kLPAREN) {
-      ErrorMsg("'(' expected");
+    if (CurrentToken() != Token::kCOMMA) {
+      break;
     }
-    SkipToMatchingParenthesis();
-  } else {
-    SkipIf(Token::kTHIS);
-    SkipIf(Token::kPERIOD);
-    ExpectIdentifier("identifier expected");
-    ExpectToken(Token::kASSIGN);
-    SetAllowFunctionLiterals(false);
-    SkipExpr();
-    SetAllowFunctionLiterals(true);
-  }
-  while (CurrentToken() == Token::kCOMMA) {
     ConsumeToken();
-    SkipIf(Token::kTHIS);
-    SkipIf(Token::kPERIOD);
-    ExpectIdentifier("instance field expected");
-    ExpectToken(Token::kASSIGN);
-    SetAllowFunctionLiterals(false);
-    SkipExpr();
-    SetAllowFunctionLiterals(true);
   }
 }