Add named constructor name 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"
 #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, constructor_name_check, false,
+            "Named constructors may not clash with other members");
 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_map_literal, false,
             "Warning on legacy map literal syntax (single type argument)");
 
 static void CheckedModeHandler(bool value) {
   FLAG_enable_asserts = value;
@@ skipping 438 matching lines @@
     has_final = false;
     has_const = false;
     has_static = false;
     has_var = false;
     has_factory = false;
     has_operator = false;
     type = NULL;
     name_pos = 0;
     name = NULL;
     redirect_name = NULL;
+    constructor_name = NULL;
     params.Clear();
     kind = RawFunction::kRegularFunction;
   }
   bool IsConstructor() const {
     return (kind == RawFunction::kConstructor) && !has_static;
   }
   bool IsFactory() const {
     return (kind == RawFunction::kConstructor) && has_static;
   }
   bool IsFactoryOrConstructor() const {
     return (kind == RawFunction::kConstructor);
   }
   bool IsGetter() const {
     return kind == RawFunction::kGetterFunction;
   }
   bool IsSetter() const {
     return kind == RawFunction::kSetterFunction;
   }
   bool has_abstract;
   bool has_external;
   bool has_final;
   bool has_const;
   bool has_static;
   bool has_var;
   bool has_factory;
   bool has_operator;
   const AbstractType* type;
   intptr_t name_pos;
   String* name;
-  String* redirect_name;  // For constructors: NULL or redirected constructor.
+  // For constructors: NULL or redirected constructor.
+  String* redirect_name;
+  // For constructors: NULL for unnamed constructor,
+  // identifier after classname for named constructors.
+  String* constructor_name;
   ParamList params;
   RawFunction::Kind kind;
 };
 
 
 class ClassDesc : public ValueObject {
  public:
   ClassDesc(const Class& cls,
             const String& cls_name,
             bool is_interface,
@@ skipping 1869 matching lines @@
       (method->params.num_fixed_parameters == 1)) {
     // Patch up name for unary operator - so it does not clash with the
     // name for binary operator -.
     *method->name = Symbols::New("unary-");
   }
 
   if (members->FunctionNameExists(*method->name, method->kind)) {
     ErrorMsg(method->name_pos,
              "field or method '%s' already defined", method->name->ToCString());
   }
+  if (FLAG_constructor_name_check && method->constructor_name != NULL) {
+    if (members->FunctionNameExists(*method->constructor_name, method->kind)) {
+      ErrorMsg(method->name_pos,
+               "Named constructor '%s' conflicts with method or field '%s'",
+               method->name->ToCString(),
+               method->constructor_name->ToCString());
+    }
+  }

[siva, 2012/09/06 16:52:10]

As discussed offline how do we report errors for the case of a field or function
appearing after the named constructor?

[hausner, 2012/09/06 18:02:13]

Correct. We really should have a simpler way to record the names in a scope. I
moved the check so it's now performed after the class is parsed. 

 
   // Mangle the name for getter and setter functions and check function
   // arity.
   if (method->IsGetter() || method->IsSetter()) {
     int expected_num_parameters = 0;
     if (method->IsGetter()) {
       expected_num_parameters = (method->has_static) ? 0 : 1;
       method->name = &String::ZoneHandle(Field::GetterSymbol(*method->name));
     } else {
       ASSERT(method->IsSetter());
@@ skipping 446 matching lines @@
       member.name_pos = TokenPos();
       member.name = CurrentLiteral();
       ConsumeToken();
     }
     // We must be dealing with a constructor or named constructor.
     member.kind = RawFunction::kConstructor;
     String& ctor_suffix = String::ZoneHandle(Symbols::Dot());
     if (CurrentToken() == Token::kPERIOD) {
       // Named constructor.
       ConsumeToken();
-      const String* name = ExpectIdentifier("identifier expected");
-      ctor_suffix = String::Concat(ctor_suffix, *name);
+      member.constructor_name = ExpectIdentifier("identifier expected");
+      ctor_suffix = String::Concat(ctor_suffix, *member.constructor_name);
     }
     *member.name = String::Concat(*member.name, ctor_suffix);
     // Ensure that names are symbols.
     *member.name = Symbols::New(*member.name);
     if (member.type == NULL) {
       ASSERT(!member.has_factory);
       // The body of the constructor cannot modify the type arguments of the
       // constructed instance, which is passed in as a hidden parameter.
       // Therefore, there is no need to set the result type to be checked.
       member.type = &Type::ZoneHandle(Type::DynamicType());
@@ skipping 6505 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart