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

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/flags.h"
 
 #ifndef DART_PRECOMPILED_RUNTIME
 
 #include "lib/invocation_mirror.h"
@@ skipping 3003 matching lines @@
   // have an initializer expression inlined in the class declaration.
   // The remaining fields are those initialized by the constructor's
   // initializing formals and initializer list
   int initializer_idx = 0;
   while (initializer_idx < initialized_fields->length()) {
     Field* initialized_field = (*initialized_fields)[initializer_idx];
     initializer_idx++;
     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.
       // Throw a NoSuchMethodError for the missing setter.
       ASSERT(field->is_final());
 
       // Build a call to NoSuchMethodError::_throwNew(
       //     Object receiver,
       //     String memberName,
       //     int invocation_type,
@@ skipping 1454 matching lines @@
     MemberDesc* existing_member = &members->members()[i];
     if (name.Equals(*existing_member->DictName())) {
       ReportError(
           member->name_pos, "%s '%s' conflicts with previously declared %s",
           member->ToCString(), name.ToCString(), existing_member->ToCString());
     }
   }
 }
 
 
+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");
   MemberDesc member;
   current_member_ = &member;
   member.metadata_pos = metadata_pos;
   member.decl_begin_pos = TokenPos();
   if ((CurrentToken() == Token::kEXTERNAL) &&
       (LookaheadToken(1) != Token::kLPAREN)) {
     ConsumeToken();
@@ skipping 460 matching lines @@
     ParseClassMemberDefinition(&members, metadata_pos);
   }
   ExpectToken(Token::kRBRACE);
 
   if (cls.LookupTypeParameter(class_name) != TypeParameter::null()) {
     ReportError(class_pos, "class name conflicts with type parameter '%s'",
                 class_name.ToCString());
   }
   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 =
       !members.has_constructor() && !cls.is_patch();
 
   cls.AddFields(members.fields());
 
   // Creating a new array for functions marks the class as parsed.
   Array& array = Array::Handle(Z, members.MakeFunctionsArray());
   cls.SetFunctions(array);
@@ skipping 10326 matching lines @@
                                                 TokenPosition* start,
                                                 TokenPosition* end) {
   UNREACHABLE();
   return false;
 }
 
 
 }  // namespace dart
 
 #endif  // DART_PRECOMPILED_RUNTIME