- Split functionality of adding implicit constructors and checking

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"
@@ skipping 2929 matching lines @@
       ErrorMsg(classname_pos, "'%s' is already defined",
                class_name.ToCString());
     }
     cls ^= obj.raw();
     if (cls.is_interface()) {
       ErrorMsg(classname_pos, "'%s' %s",
                class_name.ToCString(),
                is_patch ?
                    "is already defined as interface" :
                    "interface cannot be patched");
-    } else if (!is_patch && (cls.functions() != Object::empty_array())) {
-      ErrorMsg(classname_pos, "class '%s' is already defined",

[hausner, 2012/08/15 16:39:27]

Where is this duplicate class definition detected after your change?

-               class_name.ToCString());
     } else if (is_patch) {
       String& patch = String::Handle(Symbols::New("patch "));
       patch = String::Concat(patch, class_name);
       patch = Symbols::New(patch);
       cls = Class::New(patch, script_, classname_pos);
     } else {
       // Not patching a class, but it has been found. This must be one of the
       // pre-registered classes from object.cc or a duplicate definition.
       if (cls.functions() != Object::empty_array()) {
         ErrorMsg(classname_pos, "class '%s' is already defined",

[Ivan Posva, 2012/08/15 16:42:33]

Right here.

[hausner, 2012/08/15 17:01:09]

Ah right, where it was before patch support. Thanks.

                  class_name.ToCString());
       }
       // Pre-registered classes need their scripts connected at this time.
       cls.set_script(script_);
     }
   }
   ASSERT(!cls.IsNull());
   ASSERT(cls.functions() == Object::empty_array());
   set_current_class(cls);
   ParseTypeParameters(cls);
@@ skipping 30 matching lines @@
     AddInterfaces(interfaces_pos, cls, interfaces);
   }
 
   ExpectToken(Token::kLBRACE);
   ClassDesc members(cls, class_name, false, class_pos);
   while (CurrentToken() != Token::kRBRACE) {
     ParseClassMemberDefinition(&members);
   }
   ExpectToken(Token::kRBRACE);
 
-  if (!is_patch) {
-    // Do not install implicit constructors.
-    CheckConstructors(&members);
+  // Add an implicit constructor if no explicit constructor is present. No
+  // implicit constructors are needed for patch classes.
+  if (!members.has_constructor() && !is_patch) {
+    AddImplicitConstructor(&members);
   }
+  CheckConstructorCycles(&members);
 
   Array& array = Array::Handle();
   array = Array::MakeArray(members.fields());
   cls.SetFields(array);
 
   // Creating a new array for functions marks the class as parsed.
   array = Array::MakeArray(members.functions());
   cls.SetFunctions(array);
 
   if (!is_patch) {
     pending_classes.Add(cls, Heap::kOld);
   } else {
     // Lookup the patched class and apply the changes.
     obj = library_.LookupObject(class_name);
     Class::Cast(obj).ApplyPatch(cls);
   }
 }
 
 
-// 1. Add an implicit constructor if no explicit constructor is present.
-// 2. Check for cycles in constructor redirection.
-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 explicit parameter,
-    // and contains a supercall in the initializer list.
-    String& ctor_name = String::ZoneHandle(
-        String::Concat(class_desc->class_name(),
-                       String::Handle(Symbols::Dot())));
-    ctor_name = Symbols::New(ctor_name);
-    // The token position for the implicit constructor is the 'class'
-    // keyword of the constructor's class.
-    Function& ctor = Function::Handle(
-        Function::New(ctor_name,
-                      RawFunction::kConstructor,
-                      /* is_static = */ false,
-                      /* is_const = */ false,
-                      /* is_abstract = */ false,
-                      /* is_external = */ false,
-                      current_class(),
-                      class_desc->token_pos()));
-    ParamList params;
-    // Add implicit 'this' parameter.
-    ASSERT(current_class().raw() == ctor.Owner());
-    params.AddReceiver(ReceiverType(TokenPos()));
-    // Add implicit parameter for construction phase.
-    params.AddFinalParameter(
-        TokenPos(),
-        &String::ZoneHandle(Symbols::PhaseParameter()),
-        &Type::ZoneHandle(Type::IntInterface()));
+// Add an implicit constructor if no explicit constructor is present.
+void Parser::AddImplicitConstructor(ClassDesc* class_desc) {
+  // 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(Symbols::Dot())));
+  ctor_name = Symbols::New(ctor_name);
+  // The token position for the implicit constructor is the 'class'
+  // keyword of the constructor's class.
+  Function& ctor = Function::Handle(
+      Function::New(ctor_name,
+                    RawFunction::kConstructor,
+                    /* is_static = */ false,
+                    /* is_const = */ false,
+                    /* is_abstract = */ false,
+                    /* is_external = */ false,
+                    current_class(),
+                    class_desc->token_pos()));
+  ParamList params;
+  // Add implicit 'this' parameter.
+  ASSERT(current_class().raw() == ctor.Owner());
+  params.AddReceiver(ReceiverType(TokenPos()));
+  // Add implicit parameter for construction phase.
+  params.AddFinalParameter(
+                           TokenPos(),

[Mads Ager (google), 2012/08/15 07:21:24]

Move this argument to the line above or use 4-space indent.

[Ivan Posva, 2012/08/15 07:28:23]

Done.

+                           &String::ZoneHandle(Symbols::PhaseParameter()),
+                           &Type::ZoneHandle(Type::IntInterface()));
 
-    AddFormalParamsToFunction(&params, ctor);
-    // The body of the constructor cannot modify the type of the constructed
-    // instance, which is passed in as the receiver.
-    ctor.set_result_type(*((*params.parameters)[0].type));
-    class_desc->AddFunction(ctor);
-  }
+  AddFormalParamsToFunction(&params, ctor);
+  // The body of the constructor cannot modify the type of the constructed
+  // instance, which is passed in as the receiver.
+  ctor.set_result_type(*((*params.parameters)[0].type));
+  class_desc->AddFunction(ctor);
+}
 
+
+// Check for cycles in constructor redirection.
+void Parser::CheckConstructorCycles(ClassDesc* class_desc) {
   // Check for cycles in constructor redirection.
   const GrowableArray<MemberDesc>& members = class_desc->members();
   for (int i = 0; i < members.length(); i++) {
     MemberDesc* member = &members[i];
     GrowableArray<MemberDesc*> ctors;
     while ((member != NULL) && (member->redirect_name != NULL)) {
       ASSERT(member->IsConstructor());
       // Check whether we have already seen this member.
       for (int i = 0; i < ctors.length(); i++) {
         if (ctors[i] == member) {
@@ skipping 5997 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart