Two-phase constructors

runtime/vm/parser.cc

Index: runtime/vm/parser.cc
===================================================================
--- runtime/vm/parser.cc	(revision 1045)
+++ runtime/vm/parser.cc	(working copy)
@@ -41,6 +41,7 @@
 static const char* kStringClassName = "StringBase";
 static const char* kInterpolateName = "_interpolate";
 static const char* kThisName = "this";
+static const char* kPhaseParameterName = ":phase";
 static const char* kGetIteratorName = "iterator";
 
 #if defined(DEBUG)
@@ -1037,10 +1038,11 @@
   const Function& super_function = Function::ZoneHandle(
       GetSuperFunction(supercall_pos, function_name));
 
+  ArgumentListNode* arguments = new ArgumentListNode(supercall_pos);
   // 'this' parameter is the first argument to super call.
   AstNode* receiver = LoadReceiver(supercall_pos);
-  ArgumentListNode* arguments =
-      ParseActualParameters(receiver, kAllowConst);
+  arguments->Add(receiver);
+  ParseActualParameters(arguments, kAllowConst);
   return new StaticCallNode(supercall_pos, super_function, arguments);
 }
 
@@ -1212,7 +1214,7 @@
 }
 
 
-void Parser::GenerateSuperInitializerCall(const Class& cls,
+void Parser::GenerateSuperConstructorCall(const Class& cls,
                                           LocalVariable* receiver) {
   const intptr_t supercall_pos = token_index_;
   const Class& super_class = Class::Handle(cls.SuperClass());
@@ -1226,9 +1228,14 @@
   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.
+  // Implicit 'this' parameter is the first argument.
   AstNode* implicit_argument = new LoadLocalNode(supercall_pos, *receiver);
   arguments->Add(implicit_argument);
+  // Implicit constructor phase parameter is second argument.
+  AstNode* phase_parameter =
+      new LiteralNode(supercall_pos,
+                      Smi::ZoneHandle(Smi::New(Function::kCtorPhaseAll)));
+  arguments->Add(phase_parameter);
   const Function& super_ctor = Function::ZoneHandle(
       super_class.LookupConstructor(ctor_name));
   if (super_ctor.IsNull() ||
@@ -1263,13 +1270,24 @@
   if (CurrentToken() != Token::kLPAREN) {
     ErrorMsg("parameter list expected");
   }
+
+  ArgumentListNode* arguments = new ArgumentListNode(supercall_pos);
   // 'this' parameter is the first argument to super class constructor.
   AstNode* implicit_argument = new LoadLocalNode(supercall_pos, *receiver);
+  arguments->Add(implicit_argument);
+  // Second implicit parameter is the constructor phase. We optimistically
+  // assume that we can execute both the super initializer and the super
+  // constructor body. We may later change this to only execute the
+  // super initializer.
+  AstNode* phase_parameter =
+      new LiteralNode(supercall_pos,
+                      Smi::ZoneHandle(Smi::New(Function::kCtorPhaseAll)));
+  arguments->Add(phase_parameter);
   // 'this' parameter must not be accessible to the other super call arguments.
   receiver->set_invisible(true);
-  ArgumentListNode* arguments =
-      ParseActualParameters(implicit_argument, kAllowConst);
+  ParseActualParameters(arguments, kAllowConst);
   receiver->set_invisible(false);
+
   // Resolve the constructor.
   const Function& super_ctor = Function::ZoneHandle(
       super_class.LookupConstructor(ctor_name));
@@ -1373,10 +1391,9 @@
 }
 
 
-void Parser::ParseInitializers(const Class& cls) {
+void Parser::ParseInitializers(const Class& cls, LocalVariable* receiver) {
   TRACE_PARSER("ParseInitializers");
-  LocalVariable* receiver = current_block_->scope->VariableAt(0);
-  AstNode* super_init_statement = NULL;
+  bool super_init_seen = false;
   if (CurrentToken() == Token::kCOLON) {
     if ((LookaheadToken(1) == Token::kTHIS) &&
         ((LookaheadToken(2) == Token::kLPAREN) ||
@@ -1390,33 +1407,26 @@
       ParseConstructorRedirection(cls, receiver);
       return;
     }
-
     do {
       ConsumeToken();  // Colon or comma.
+      AstNode* init_statement;
       if (CurrentToken() == Token::kSUPER) {
-        if (super_init_statement != NULL) {
+        if (super_init_seen) {
           ErrorMsg("Duplicate call to super constructor");
         }
-        super_init_statement = ParseSuperInitializer(cls, receiver);
+        init_statement = ParseSuperInitializer(cls, receiver);
+        super_init_seen = true;
       } else {
-        AstNode* init_statement = ParseInitializer(cls, receiver);
-        current_block_->statements->Add(init_statement);
+        init_statement = ParseInitializer(cls, receiver);
       }
+      current_block_->statements->Add(init_statement);
     } while (CurrentToken() == Token::kCOMMA);
   }
-
-  if (super_init_statement != NULL) {
-    // Move explicit supercall to the end of the initializer list to
-    // avoid executing constructor code on partially initialized objects.
-    // TODO(hausner): Fix issue 4995181, evaluation order of constructor
-    // initializer lists.
-    current_block_->statements->Add(super_init_statement);
-  } else {
+  if (!super_init_seen) {
     // Generate implicit super() if we haven't seen an explicit super call
     // or constructor redirection.
-    GenerateSuperInitializerCall(cls, receiver);
+    GenerateSuperConstructorCall(cls, receiver);
   }
-
   CheckConstFieldsInitialized(cls);
 }
 
@@ -1438,10 +1448,17 @@
   if (CurrentToken() != Token::kLPAREN) {
     ErrorMsg("parameter list expected");
   }
-  // 'this' parameter is the first argument to super class constructor.
+
+  ArgumentListNode* arguments = new ArgumentListNode(call_pos);
+  // 'this' parameter is the first argument to constructor.
   AstNode* implicit_argument = new LoadLocalNode(call_pos, *receiver);
-  ArgumentListNode* arguments =
-      ParseActualParameters(implicit_argument, kAllowConst);
+  arguments->Add(implicit_argument);
+  // Constructor phase parameter is second argument.
+  LocalVariable* phase_param = LookupPhaseParameter();
+  ASSERT(phase_param != NULL);
+  AstNode* phase_argument = new LoadLocalNode(call_pos, *phase_param);
+  arguments->Add(phase_argument);
+  ParseActualParameters(arguments, kAllowConst);
 
   // Resolve the constructor.
   const Function& redirect_ctor = Function::ZoneHandle(
@@ -1465,7 +1482,6 @@
   // Implicit 'this' is the only parameter/local variable.
   OpenFunctionBlock(func);
 
-
   // Parse expressions of instance fields that have an explicit
   // initializers.
   GrowableArray<FieldInitExpression> initializers;
@@ -1478,6 +1494,12 @@
       Type::ZoneHandle(Type::DynamicType()));
   current_block_->scope->AddVariable(receiver);
 
+  LocalVariable* phase_parameter = new LocalVariable(
+       ctor_pos,
+       String::ZoneHandle(String::NewSymbol(kPhaseParameterName)),
+       Type::ZoneHandle(Type::DynamicType()));
+  current_block_->scope->AddVariable(phase_parameter);
+
   // Now that the "this" parameter is in scope, we can generate the code
   // to strore the initializer expressions in the respective instance fields.
   for (int i = 0; i < initializers.length(); i++) {
@@ -1491,7 +1513,7 @@
     current_block_->statements->Add(field_init);
   }
 
-  GenerateSuperInitializerCall(cls, receiver);
+  GenerateSuperConstructorCall(cls, receiver);
   CheckConstFieldsInitialized(cls);
 
   // Empty constructor body.
@@ -1531,6 +1553,11 @@
   if (has_receiver) {
     params.AddReceiver(token_index_);
   }
+  if (func.IsConstructor()) {
+    // Add implicit parameter for constructor phase.
+    params.AddFinalParameter(token_index_, kPhaseParameterName,
+                             &Type::ZoneHandle(Type::DynamicType()));
+  }
   if (are_implicitly_final) {
     params.SetImplicitlyFinal();
   }
@@ -1551,70 +1578,95 @@
   // NB: the instance field initializers have to be compiled before
   // the parameters are added to the scope, so that a parameter
   // name cannot shadow a name used in the field initializer expression.
-
-  GrowableArray<FieldInitExpression> initializers;
+  SequenceNode* init_statements = NULL;
   if (func.IsConstructor()) {

[srdjan, 2011/11/02 00:08:13]

The if (func.IsConstructor()) is very long. Can you factor it out to a separate
method?

[hausner, 2011/11/02 00:26:40]

I am planning to separate the compilation of constructors and regular functions
in a subsequent change list.

+    GrowableArray<FieldInitExpression> initializers;
     ParseInitializedInstanceFields(cls, &initializers);
-  }
 
-  // Now populate function scope with the formal parameters.
-  AddFormalParamsToScope(&params, current_block_->scope);
+    // Now populate function scope with the formal parameters.
+    AddFormalParamsToScope(&params, current_block_->scope);
+    LocalVariable* receiver = current_block_->scope->VariableAt(0);
 
-  // Now that the "this" parameter is in scope, we can generate the code
-  // to strore the initializer expressions in the respective instance fields.
-  // We do this before the field parameters and the initializers from the
-  // constuctor's initializer list get compiled.
-  if (initializers.length() > 0) {
-    LocalVariable* receiver = current_block_->scope->VariableAt(0);
-    for (int i = 0; i < initializers.length(); i++) {
-      const Field* field = initializers[i].inst_field;
-      AstNode* instance = new LoadLocalNode(field->token_index(), *receiver);
-      AstNode* field_init =
-          new StoreInstanceFieldNode(field->token_index(),
-                                     instance,
-                                     *field,
-                                     initializers[i].expr);
-      current_block_->statements->Add(field_init);
+    // Now that the "this" parameter is in scope, we can generate the code
+    // to strore the initializer expressions in the respective instance fields.
+    // We do this before the field parameters and the initializers from the
+    // constuctor's initializer list get compiled.

[srdjan, 2011/11/02 00:08:13]

constructor's

[hausner, 2011/11/02 00:26:40]

Done.

+    OpenBlock();
+    if (initializers.length() > 0) {
+      for (int i = 0; i < initializers.length(); i++) {
+        const Field* field = initializers[i].inst_field;
+        AstNode* instance = new LoadLocalNode(field->token_index(), *receiver);
+        AstNode* field_init =
+            new StoreInstanceFieldNode(field->token_index(),
+                                       instance,
+                                       *field,
+                                       initializers[i].expr);
+        current_block_->statements->Add(field_init);
+      }
     }
-  }
 
-  // Turn formal field parameters into field initializers or report error
-  // if the function is not a constructor
-  if (params.has_field_initializer) {
-    LocalVariable* receiver = current_block_->scope->VariableAt(0);
-    for (int i = 0; i < params.parameters->length(); i++) {
-      ParamDesc& param = (*params.parameters)[i];
-      if (param.is_field_initializer) {
-        if (!func.IsConstructor()) {
+    // Turn formal field parameters into field initializers or report error
+    // if the function is not a constructor
+    if (params.has_field_initializer) {
+      for (int i = 0; i < params.parameters->length(); i++) {
+        ParamDesc& param = (*params.parameters)[i];
+        if (param.is_field_initializer) {
+          if (!func.IsConstructor()) {
+            ErrorMsg(param.name_pos,
+                     "field initializer only allowed in constructors");
+          }
+
+          const String& field_name = *param.name;
+          Field& field = Field::ZoneHandle(cls.LookupInstanceField(field_name));
+          if (field.IsNull()) {
+            ErrorMsg(param.name_pos,
+                     "unresolved reference to instance field '%s'",
+                     field_name.ToCString());
+          }
+          const String& mangled_name =
+              String::ZoneHandle(MangledInitParamName(field_name));
+          AstNode* instance = new LoadLocalNode(param.name_pos, *receiver);
+          LocalVariable* p =
+              current_block_->scope->LookupVariable(mangled_name, false);
+          ASSERT(p != NULL);
+          AstNode* value = new LoadLocalNode(param.name_pos, *p);
+          AstNode* initializer = new StoreInstanceFieldNode(
+              param.name_pos, instance, field, value);
+          current_block_->statements->Add(initializer);
+        }
+      }
+    }
+    ParseInitializers(cls, receiver);
+    init_statements = CloseBlock();
+    LocalVariable* phase_param = LookupPhaseParameter();
+    AstNode* phase_value = new LoadLocalNode(token_index_, *phase_param);
+    AstNode* phase_check =
+        new BinaryOpNode(token_index_, Token::kBIT_AND,
+                phase_value,
+                new LiteralNode(token_index_,
+                        Smi::ZoneHandle(Smi::New(Function::kCtorPhaseInit))));
+    AstNode* comparison =
+        new ComparisonNode(token_index_, Token::kNE_STRICT,
+                phase_check,
+                new LiteralNode(token_index_, Smi::ZoneHandle(Smi::New(0))));
+    AstNode* guarded_init_statements =
+        new IfNode(token_index_, comparison, init_statements, NULL);
+    current_block_->statements->Add(guarded_init_statements);
+  } else {
+    // Parsing a function that is not a constructor.
+    if (params.has_field_initializer) {
+      for (int i = 0; i < params.parameters->length(); i++) {
+        ParamDesc& param = (*params.parameters)[i];
+        if (param.is_field_initializer) {
           ErrorMsg(param.name_pos,
                    "field initializer only allowed in constructors");
         }
-
-        const String& field_name = *param.name;
-        Field& field = Field::ZoneHandle(cls.LookupInstanceField(field_name));
-        if (field.IsNull()) {
-          ErrorMsg(param.name_pos,
-                   "unresolved reference to instance field '%s'",
-                   field_name.ToCString());
-        }
-        const String& mangled_name =
-            String::ZoneHandle(MangledInitParamName(field_name));
-        AstNode* instance = new LoadLocalNode(param.name_pos, *receiver);
-        LocalVariable* p =
-            current_block_->scope->LocalLookupVariable(mangled_name);
-        ASSERT(p != NULL);
-        AstNode* value = new LoadLocalNode(param.name_pos, *p);
-        AstNode* initializer =
-            new StoreInstanceFieldNode(param.name_pos, instance, field, value);
-        current_block_->statements->Add(initializer);
       }
     }
+    // Populate function scope with the formal parameters.
+    AddFormalParamsToScope(&params, current_block_->scope);
   }
 
-  if (func.IsConstructor()) {
-    ParseInitializers(cls);
-  }
-
   if (FLAG_enable_type_checks &&
       (current_block_->scope->function_level() > 0)) {
     // We are parsing, but not compiling, a local function.
@@ -1630,6 +1682,87 @@
     }
   }
 
+  if (func.IsConstructor()) {

[srdjan, 2011/11/02 00:08:13]

Also a very  long if.

[hausner, 2011/11/02 00:26:40]

see above.

+    LocalVariable* receiver = current_block_->scope->VariableAt(0);
+    StaticCallNode* super_call = NULL;
+    ASSERT(init_statements != NULL);
+    // Look for the super initializer call in the sequence of initializer
+    // statements. If it exists and is not the last initializer statement,
+    // we need to create an implicit super call to the super constructor's
+    // body.
+    // Thus, iterate over all but the last initializer to see whether
+    // it's a super constructor call.
+    for (int i = 0; i < init_statements->length() - 1; i++) {
+      if (init_statements->NodeAt(i)->IsStaticCallNode()) {
+        StaticCallNode* static_call =
+            init_statements->NodeAt(i)->AsStaticCallNode();
+        if (static_call->function().IsConstructor()) {
+          super_call = static_call;
+          break;
+        }
+      }
+    }
+    if (super_call != NULL) {
+      // Generate an implicit call to the super constructor's body.
+      // We need to patch the super _initializer_ call so that it
+      // saves the evaluated actual arguments in temporary variables.
+      // The temporary variables are necessary so that the argument
+      // expressions are not evaluated twice.
+      ArgumentListNode* ctor_args = super_call->arguments();
+      // The super initializer call has at least 2 arguments: the
+      // implicit receiver, and the hidden constructor phase.
+      ASSERT(ctor_args->length() >= 2);
+      for (int i = 2; i < ctor_args->length(); i++) {
+        AstNode* arg = ctor_args->NodeAt(i);
+        if (!arg->IsLoadLocalNode() && !arg->IsLiteralNode()) {
+          LocalVariable* temp =
+              CreateTempConstVariable(arg->token_index(), arg->id(), "sca");
+          AstNode* save_temp =
+              new StoreLocalNode(arg->token_index(), *temp, arg);
+          ctor_args->SetNodeAt(i, save_temp);
+        }
+      }
+    }
+    OpenBlock();
+    if (super_call != NULL) {
+      ArgumentListNode* initializer_args = super_call->arguments();
+      const Function& super_ctor = super_call->function();
+      // Patch the initializer call so it only executes the super
+      // initializer.
+      initializer_args->SetNodeAt(1,
+          new LiteralNode(token_index_,
+                  Smi::ZoneHandle(Smi::New(Function::kCtorPhaseInit))));
+
+      ArgumentListNode* super_call_args = new ArgumentListNode(token_index_);
+      // First argument is the receiver.
+      super_call_args->Add(new LoadLocalNode(token_index_, *receiver));
+      // Second argument is the constructor phase argument.
+      AstNode* phase_parameter =
+          new LiteralNode(token_index_,
+               Smi::ZoneHandle(Smi::New(Function::kCtorPhaseBody)));
+      super_call_args->Add(phase_parameter);
+      for (int i = 2; i < initializer_args->length(); i++) {
+        AstNode* arg = initializer_args->NodeAt(i);
+        if (arg->IsLiteralNode()) {
+          LiteralNode* lit = arg->AsLiteralNode();
+          super_call_args->Add(new LiteralNode(token_index_, lit->literal()));
+        } else {
+          ASSERT(arg->IsLoadLocalNode() || arg->IsStoreLocalNode());
+          if (arg->IsLoadLocalNode()) {
+            const LocalVariable& temp = arg->AsLoadLocalNode()->local();
+            super_call_args->Add(new LoadLocalNode(token_index_, temp));
+          } else if (arg->IsStoreLocalNode()) {
+            const LocalVariable& temp = arg->AsStoreLocalNode()->local();
+            super_call_args->Add(new LoadLocalNode(token_index_, temp));
+          }
+        }
+      }
+      ASSERT(super_ctor.AreValidArguments(super_call_args->length(),
+                                          super_call_args->names()));
+      current_block_->statements->Add(
+          new StaticCallNode(token_index_, super_ctor, super_call_args));
+    }
+  }
   if (CurrentToken() == Token::kLBRACE) {
     ConsumeToken();
     ParseStatementSequence();
@@ -1649,6 +1782,23 @@
   } else {
     UnexpectedToken();
   }
+  if (func.IsConstructor()) {
+    SequenceNode* ctor_block = CloseBlock();
+    LocalVariable* phase_param = LookupPhaseParameter();
+    AstNode* phase_value = new LoadLocalNode(token_index_, *phase_param);
+    AstNode* phase_check =
+        new BinaryOpNode(token_index_, Token::kBIT_AND,
+                phase_value,
+                new LiteralNode(token_index_,
+                        Smi::ZoneHandle(Smi::New(Function::kCtorPhaseBody))));
+    AstNode* comparison =
+    new ComparisonNode(token_index_, Token::kNE_STRICT,
+            phase_check,
+            new LiteralNode(token_index_, Smi::ZoneHandle(Smi::New(0))));
+    AstNode* guarded_block_statements =
+        new IfNode(token_index_, comparison, ctor_block, NULL);
+    current_block_->statements->Add(guarded_block_statements);
+  }
 
   SequenceNode* statements = CloseBlock();
   return statements;
@@ -1796,6 +1946,11 @@
   if (has_this_param) {
     method->params.AddReceiver(formal_param_pos);
   }
+  // Constructors have an implicit parameter for the constructor phase.
+  if (method->IsConstructor()) {
+    method->params.AddFinalParameter(token_index_, kPhaseParameterName,
+                                     &Type::ZoneHandle(Type::DynamicType()));
+  }
   if (are_implicitly_final) {
     method->params.SetImplicitlyFinal();
   }
@@ -2307,8 +2462,8 @@
 void Parser::CheckConstructors(ClassDesc* class_desc) {
   // Add an implicit constructor if no explicit constructor is present.
   if (!class_desc->has_constructor()) {
-    // The implicit constructor is unnamed, has no parameter, and contains
-    // a supercall in the initializer list.
+    // The implicit constructor is unnamed, has no explicit parameter,
+    // and contains a supercall in the initializer list.
     String& ctor_name = String::ZoneHandle(
         String::Concat(class_desc->class_name(),
                        String::Handle(String::NewSymbol("."))));
@@ -2320,7 +2475,12 @@
                       /* is_const = */ false,
                       class_desc->token_pos()));
     ParamList params;
+    // Add implicit 'this' parameter.
     params.AddReceiver(token_index_);
+    // Add implicit parameter for constructor phase.
+    params.AddFinalParameter(token_index_, kPhaseParameterName,
+                             &Type::ZoneHandle(Type::DynamicType()));
+
     AddFormalParamsToFunction(&params, ctor);
     // TODO(regis): What are the type arguments?
     Type& result_type = Type::ZoneHandle(
@@ -3200,6 +3360,14 @@
 }
 
 
+LocalVariable* Parser::LookupPhaseParameter() {
+  const String& phase_name =
+      String::Handle(String::NewSymbol(kPhaseParameterName));
+  const bool kTestOnly = false;
+  return current_block_->scope->LookupVariable(phase_name, kTestOnly);
+}
+
+
 void Parser::CaptureReceiver() {
   ASSERT(current_block_->scope->function_level() > 0);
   const bool kTestOnly = false;
@@ -5200,14 +5368,17 @@
 }
 
 
-ArgumentListNode* Parser::ParseActualParameters(AstNode* implicit_argument,
-                                                bool require_const) {
+ArgumentListNode* Parser::ParseActualParameters(
+                              ArgumentListNode* implicit_arguments,
+                              bool require_const) {
   TRACE_PARSER("ParseActualParameters");
   ASSERT(CurrentToken() == Token::kLPAREN);
   const bool saved_mode = SetAllowFunctionLiterals(true);
-  ArgumentListNode* arguments = new ArgumentListNode(token_index_);
-  if (implicit_argument != NULL) {
-    arguments->Add(implicit_argument);
+  ArgumentListNode* arguments;
+  if (implicit_arguments == NULL) {
+    arguments = new ArgumentListNode(token_index_);
+  } else {
+    arguments = implicit_arguments;
   }
   GrowableArray<const String*> names;
   bool named_argument_seen = false;
@@ -5807,7 +5978,7 @@
     const TypeArguments& type_arguments,
     const Function& constructor,
     ArgumentListNode* arguments) {
-  GrowableArray<const Object*> arg_values(arguments->length() + 1);
+  GrowableArray<const Object*> arg_values(arguments->length() + 2);

[srdjan, 2011/11/02 00:08:13]

Please add comment about + 2

[hausner, 2011/11/02 00:26:40]

Done.

   Instance& instance = Instance::Handle();
   if (!constructor.IsFactory()) {
     instance = Instance::New(type_class);
@@ -5819,6 +5990,7 @@
       instance.SetTypeArguments(type_arguments);
     }
     arg_values.Add(&instance);
+    arg_values.Add(&Smi::ZoneHandle(Smi::New(Function::kCtorPhaseAll)));
   } else {
     // Prepend type_arguments to list of arguments to factory.
     ASSERT(type_arguments.IsZoneHandle());
@@ -6483,7 +6655,8 @@
 
   // A constructor has an implicit 'this' parameter (instance to construct)
   // and a factory has an implicit 'this' parameter (type_arguments).
-  intptr_t arguments_length = arguments->length() + 1;
+  // A constructor has a second implicit 'phase' parameter.
+  intptr_t arguments_length = arguments->length() + 2;
 
   if (type_class.is_interface()) {
     // We need to make sure that an appropriate constructor is
@@ -6548,6 +6721,8 @@
       type_class.LookupConstructor(constructor_name));
   if (constructor.IsNull()) {
     constructor = type_class.LookupFactory(constructor_name);
+    // A factory does not have the implicit 'phase' parameter.
+    arguments_length -= 1;
   }
   if (constructor.IsNull()) {
     ErrorMsg(new_pos, "class '%s' has no constructor or factory named '%s'",