Updated parser to produce error when trying to reinitialize final variables in class constructors. …

runtime/vm/parser.cc

Index: runtime/vm/parser.cc
diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc
index f96c1a3ec17e9fb14636824aff397baba1ee81ab..fe4ae4cd0266f6ea1836e1a39c32551ffd740ec5 100644
--- a/runtime/vm/parser.cc
+++ b/runtime/vm/parser.cc
@@ -3021,6 +3021,14 @@ AstNode* Parser::CheckDuplicateFieldInit(
     if (initialized_field == NULL) {
       break;
     }
+
+    const String& initialized_name = String::Handle(initialized_field->name());
+    const String& field_name = String::Handle(field->name());
+    if (initialized_name.Equals(field_name) && field->has_initializer()) {
+      ReportError(init_pos, "final field '%s' is already initialized.",
+                  field_name.ToCString());
+    }

[siva, 2017/06/16 00:51:10]

Pull the handle creation outside the while loop

String& initialized_name = String::Handle(Z);
String field_name = String::Handle(Z);

while (initialize.....) {
  ...
  ...
  initialized_name ^= initialized_field->name();
  field_name ^= field->name();
  if (...) {
    ...
  }
  ....
  ....

[bkonyi, 2017/06/16 19:40:44]

Done.

+
     if (initialized_field->raw() == field->raw()) {
       // This final field has been initialized by an inlined
       // initializer expression. This is a runtime error.
@@ -4495,6 +4503,42 @@ void Parser::CheckMemberNameConflict(ClassDesc* members, MemberDesc* member) {
 }
 
 
+void Parser::CheckFinalInitializationConflicts(ClassDesc* class_desc) {
+  const GrowableArray<const Field*>& fields = class_desc->fields();
+  const GrowableArray<MemberDesc>& members = class_desc->members();
+  for (intptr_t i = 0; i < fields.length(); i++) {
+    const Field* current_field = fields.At(i);
+    if (!current_field->is_final() || !current_field->has_initializer()) {
+      continue;
+    }
+    const String& field_name = String::Handle(current_field->name());
+    for (intptr_t j = 0; j < members.length(); j++) {
+      const MemberDesc current_member = members.At(j);
+      if (!current_member.IsConstructor()) {
+        continue;
+      }
+      const ParamList* params = &current_member.params;
+      if (!params->has_field_initializer) {
+        continue;
+      }
+      const ZoneGrowableArray<ParamDesc>& parameters = *params->parameters;
+      for (intptr_t p = 0; p < parameters.length(); p++) {
+        const ParamDesc& current_param = parameters[p];
+        if (!current_param.is_field_initializer) {
+          continue;
+        }
+        const String& param_name = *current_param.name;
+        if (param_name.Equals(field_name)) {
+          ReportError(current_param.name_pos,
+                      "final field '%s' is already initialized.",
+                      param_name.ToCString());
+        }
+      }
+    }
+  }
+}

[siva, 2017/06/16 00:51:10]

If you invert this loop as

for (intptr_t j = 0; j < members.length(); j++) {
  ....
  if (current_member.IsConstructor()) {
    ....
    ....
    for (intptr_t p = 0; ....) {
      for (intptr k = 0; k < fields.length(); k++) {
        check for field error;
      }
    }
}

Then it might be possible to merge this into Parser::CheckConstructors which is
also looping over all the members to determine if we get cycles because of
redirection.
  

[bkonyi, 2017/06/16 19:40:44]

Done.

+
+
 void Parser::ParseClassMemberDefinition(ClassDesc* members,
                                         TokenPosition metadata_pos) {
   TRACE_PARSER("ParseClassMemberDefinition");
@@ -4975,6 +5019,10 @@ void Parser::ParseClassDefinition(const Class& cls) {
   }
   CheckConstructors(&members);
 
+  // Check that our constructors don't try to reinitialize an initialized
+  // final variable.
+  CheckFinalInitializationConflicts(&members);
+
   // Need to compute this here since MakeArray() will clear the
   // functions array in members.
   const bool need_implicit_constructor =