VM: Fix bug with evaluation order of static setter invocation.

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 "lib/invocation_mirror.h"
 #include "vm/bigint_operations.h"
 #include "vm/bootstrap.h"
 #include "vm/class_finalizer.h"
@@ skipping 8118 matching lines @@
       }
 
       // Explicit setter function for the field found, field does not exist.
       // Create a getter node first in case it is needed. If getter node
       // is used as part of, e.g., "+=", and the explicit getter does not
       // exist, and error will be reported by the code generator.
       access = new StaticGetterNode(call_pos,
                                     NULL,
                                     false,
                                     Class::ZoneHandle(cls.raw()),
-                                    String::ZoneHandle(field_name.raw()));
+                                    field_name);

[Florian Schneider, 2013/10/03 15:33:22]

field_name is already a ZoneHandle.

     } else {
       // Field exists.
       if (field.is_final()) {
         // Field has been marked as final, report an error as the field
         // is not settable.
         ErrorMsg(ident_pos,
                  "field '%s' is const static, cannot assign to it",
                  field_name.ToCString());
       }
       access = GenerateStaticFieldLookup(field, TokenPos());
@@ skipping 49 matching lines @@
     if (primary->IsSuper()) {
       return primary;
     }
     // In a static method, evaluation of an unresolved identifier causes a
     // NoSuchMethodError to be thrown.
     // In an instance method, we convert this into a getter call
     // for a field (which may be defined in a subclass.)
     String& name = String::CheckedZoneHandle(primary->primary().raw());
     if (current_function().is_static() ||
         current_function().IsInFactoryScope()) {
-      return ThrowNoSuchMethodError(primary->token_pos(),
-                                    current_class(),
-                                    name,
-                                    NULL,  // No arguments.
-                                    InvocationMirror::kStatic,

[Ivan Posva, 2013/10/03 16:44:05]

You are losing information here about the kind of NoSuchMethod. Why don't we
just pass the arguments here?

[Florian Schneider, 2013/10/04 09:10:28]

We would need the RHS of the assignment as arguments here. I think this would be
a bigger change to the parser.

Actually, the contrary is the case: Here we don't know if it will be a getter or
setter. Later, the StaticGetterNode is transformed into StaticSetterNode if it
occurs in an assignment. As a result, the error message is more precise with my
change: It include the arguments and distinguishes between getter and setter.

Before:
No top-level getter or setter 'unknown' declared.

NoSuchMethodError : method not found: 'unknown'
Receiver: top-level
Arguments: [...]


After:
No top-level setter 'set:unknown' declared.

NoSuchMethodError : method not found: 'set:unknown'
Receiver: top-level
Arguments: [null]

-                                    InvocationMirror::kField,
-                                    NULL);  // No existing function.
+      return new StaticGetterNode(primary->token_pos(),
+                                  NULL,  // No receiver.
+                                  false,  // Not a super getter.
+                                  Class::ZoneHandle(current_class().raw()),
+                                  name);
     } else {
       AstNode* receiver = LoadReceiver(primary->token_pos());
       return CallGetter(node->token_pos(), receiver, name);
     }
   }
   return primary;
 }
 
 
 AstNode* Parser::LoadClosure(PrimaryNode* primary) {
@@ skipping 2394 matching lines @@
 void Parser::SkipQualIdent() {
   ASSERT(IsIdentifier());
   ConsumeToken();
   if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();  // Consume the kPERIOD token.
     ExpectIdentifier("identifier expected after '.'");
   }
 }
 
 }  // namespace dart