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::ZoneHandle(initialized_field->name());
+    const String& field_name = String::ZoneHandle(field->name());
+    if (initialized_name.Equals(field_name) && field->IsFinalAndInitialized()) {
+      ReportError(init_pos, "final field '%s' is already initialized.",
+                  field_name.ToCString());
+    }
+
     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::CheckMemberAlreadyInitializedConflicts(ClassDesc* members,
+                                                    MemberDesc* member) {
+  if (!member->IsConstructor()) {
+    return;
+  }
+  ParamList* params = &member->params;
+  if (!params->has_field_initializer) {
+    return;
+  }
+  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& name = *current_param.name;
+    for (intptr_t i = 0; i < members->members().length(); i++) {
+      MemberDesc* existing_member = &members->members()[i];
+      if (existing_member->has_final &&
+          name.Equals(*existing_member->DictName()) &&
+          (existing_member->field_ != NULL) &&
+          existing_member->field_->IsFinalAndInitialized()) {

[siva, 2017/06/13 22:37:57]

The correct check here is to use field_->has_initializer() as we have delayed
initialization of fields if the expression is not a simple literal.

[bkonyi, 2017/06/15 22:32:48]

Ah, don't know how I missed that... done.

+        ReportError(current_param.name_pos,
+                    "final field '%s' is already initialized.",
+                    existing_member->DictName()->ToCString());
+      }
+    }
+  }
+}
+
+
 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 192 matching lines @@
       AbstractType& type = AbstractType::ZoneHandle(Z, member.type->raw());
       ResolveType(&type);
       member.type = &type;
     }
     ParseFieldDefinition(members, &member);
   } else {
     UnexpectedToken();
   }
   current_member_ = NULL;
   CheckMemberNameConflict(members, &member);
+  CheckMemberAlreadyInitializedConflicts(members, &member);

[siva, 2017/06/13 22:37:56]

I don't think this is quite correct, consider this example where the final field
comes after the constructor.

class Foo {
  Foo(this.x);
  final int x = 10;
}

[bkonyi, 2017/06/15 22:32:48]

This seemed to work, but it probably makes more sense to do this after the
entire class is processed anyways.

   members->AddMember(member);
 }
 
 
 void Parser::ParseEnumDeclaration(const GrowableObjectArray& pending_classes,
                                   const Object& tl_owner,
                                   TokenPosition metadata_pos) {
   TRACE_PARSER("ParseEnumDeclaration");
   const TokenPosition declaration_pos =
       (metadata_pos.IsReal()) ? metadata_pos : TokenPos();
@@ skipping 10594 matching lines @@
                                                 TokenPosition* start,
                                                 TokenPosition* end) {
   UNREACHABLE();
   return false;
 }
 
 
 }  // namespace dart
 
 #endif  // DART_PRECOMPILED_RUNTIME