Refactor resolution code in the vm to properly handle ambiguity errors.

runtime/vm/dart_api_impl.cc

 // Copyright (c) 2013, 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 "include/dart_api.h"
 #include "include/dart_mirrors_api.h"
 #include "include/dart_native_api.h"
 
 #include "platform/assert.h"
 #include "vm/bigint_operations.h"
@@ skipping 39 matching lines @@
   } else {
     return func;
   }
 }
 
 
 static RawInstance* GetListInstance(Isolate* isolate, const Object& obj) {
   if (obj.IsInstance()) {
     const Library& core_lib = Library::Handle(Library::CoreLibrary());
     const Class& list_class =
-        Class::Handle(core_lib.LookupClass(Symbols::List()));
+        Class::Handle(core_lib.LookupClass(Symbols::List(), NULL));
     ASSERT(!list_class.IsNull());
     const Instance& instance = Instance::Cast(obj);
     const Class& obj_class = Class::Handle(isolate, obj.clazz());
     Error& malformed_type_error = Error::Handle(isolate);
     if (obj_class.IsSubtypeOf(TypeArguments::Handle(isolate),
                               list_class,
                               TypeArguments::Handle(isolate),
                               &malformed_type_error)) {
       ASSERT(malformed_type_error.IsNull());  // Type is a raw List.
       return instance.raw();
@@ skipping 972 matching lines @@
   const String& function_name =
       String::Handle(isolate_lib.PrivateName(Symbols::_get_or_create()));
   const int kNumArguments = 1;
   const Function& function = Function::Handle(
       isolate,
       Resolver::ResolveStatic(isolate_lib,
                               class_name,
                               function_name,
                               kNumArguments,
                               Object::empty_array(),
-                              Resolver::kIsQualified));
+                              Resolver::kIsQualified,
+                              NULL));  // No ambiguity error expected.
   ASSERT(!function.IsNull());
   const Array& args = Array::Handle(isolate, Array::New(kNumArguments));
   args.SetAt(0, Integer::Handle(isolate, Integer::New(port_id)));
   return Api::NewHandle(isolate, DartEntry::InvokeFunction(function, args));
 }
 
 
 DART_EXPORT Dart_Port Dart_GetMainPortId() {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
@@ skipping 976 matching lines @@
   const String& lib_url = String::Handle(String::New("dart:core"));
   const String& class_name = String::Handle(String::New("ArgumentError"));
   const Library& lib =
       Library::Handle(isolate, Library::LookupLibrary(lib_url));
   if (lib.IsNull()) {
     const String& message = String::Handle(
         String::NewFormatted("%s: library '%s' not found.",
                              CURRENT_FUNC, lib_url.ToCString()));
     return ApiError::New(message);
   }
-  const Class& cls = Class::Handle(isolate,
-                                   lib.LookupClassAllowPrivate(class_name));
+  const Class& cls = Class::Handle(
+      isolate, lib.LookupClassAllowPrivate(class_name, NULL));

[siva, 2013/07/22 22:21:46]

As discussed this should probably just ASSERT(!cls.IsNull());

[regis, 2013/07/22 23:51:27]

Done.

   if (cls.IsNull()) {
     const String& message = String::Handle(
         String::NewFormatted("%s: class '%s' not found in library '%s'.",
                              CURRENT_FUNC, class_name.ToCString(),
                              lib_url.ToCString()));
     return ApiError::New(message);
   }
   Object& result = Object::Handle(isolate);
   String& dot_name = String::Handle(String::New("."));
   String& constr_name = String::Handle(String::Concat(class_name, dot_name));
@@ skipping 335 matching lines @@
   return Dart_TypedData_kInvalid;
 }
 
 
 static RawObject* GetByteDataConstructor(Isolate* isolate,
                                          const String& constructor_name,
                                          intptr_t num_args) {
   const Library& lib =
       Library::Handle(isolate->object_store()->typed_data_library());
   ASSERT(!lib.IsNull());
-  const Class& cls =
-      Class::Handle(isolate, lib.LookupClassAllowPrivate(Symbols::ByteData()));
+  const Class& cls = Class::Handle(
+      isolate, lib.LookupClassAllowPrivate(Symbols::ByteData(), NULL));
   ASSERT(!cls.IsNull());
   return ResolveConstructor(CURRENT_FUNC,
                             cls,
                             Symbols::ByteData(),
                             constructor_name,
                             num_args);
 }
 
 
 static Dart_Handle NewByteData(Isolate* isolate, intptr_t length) {
@@ skipping 568 matching lines @@
   } else if (obj.IsLibrary()) {
     // Check whether class finalization is needed.
     const Library& lib = Library::Cast(obj);
 
     // Finalize all classes if needed.
     Dart_Handle state = Api::CheckIsolateState(isolate);
     if (::Dart_IsError(state)) {
       return state;
     }
 
+    String& ambiguity_error_msg = String::Handle(isolate);
     Function& function = Function::Handle(isolate);
-    function = lib.LookupFunctionAllowPrivate(function_name);
+    function = lib.LookupFunctionAllowPrivate(function_name,
+                                              &ambiguity_error_msg);
     if (function.IsNull()) {
+      if (!ambiguity_error_msg.IsNull()) {
+        return Api::NewError("%s.", ambiguity_error_msg.ToCString());
+      }
       return Api::NewError("%s: did not find top-level function '%s'.",
                            CURRENT_FUNC,
                            function_name.ToCString());
     }
     // LookupFunctionAllowPrivate does not check argument arity, so we
     // do it here.
     String& error_message = String::Handle();
     if (!function.AreValidArgumentCounts(number_of_arguments,
                                          0,
                                          &error_message)) {
@@ skipping 138 matching lines @@
                                                           getter_name,
                                                           args,
                                                           args_descriptor));
     }
     return Api::NewHandle(isolate, DartEntry::InvokeFunction(getter, args));
 
   } else if (obj.IsLibrary()) {
     // To access a top-level we may need to use the Field or the
     // getter Function.  The getter function may either be in the
     // library or in the field's owner class, depending.
+    String& ambiguity_error_msg = String::Handle(isolate);
     const Library& lib = Library::Cast(obj);
-    field = lib.LookupFieldAllowPrivate(field_name);
-    if (field.IsNull()) {
-      // No field found.  Check for a getter in the lib.
+    field = lib.LookupFieldAllowPrivate(field_name, &ambiguity_error_msg);
+    if (field.IsNull() && ambiguity_error_msg.IsNull()) {
+      // No field found and no ambiguity error.  Check for a getter in the lib.
       const String& getter_name =
           String::Handle(isolate, Field::GetterName(field_name));
-      getter = lib.LookupFunctionAllowPrivate(getter_name);
-    } else if (FieldIsUninitialized(isolate, field)) {
+      getter = lib.LookupFunctionAllowPrivate(getter_name,
+                                              &ambiguity_error_msg);
+    } else if (!field.IsNull() && FieldIsUninitialized(isolate, field)) {
       // A field was found.  Check for a getter in the field's owner classs.
       const Class& cls = Class::Handle(isolate, field.owner());
       const String& getter_name =
           String::Handle(isolate, Field::GetterName(field_name));
       getter = cls.LookupStaticFunctionAllowPrivate(getter_name);
     }
 
     if (!getter.IsNull()) {
       // Invoke the getter and return the result.
       return Api::NewHandle(
           isolate, DartEntry::InvokeFunction(getter, Object::empty_array()));
-    } else if (!field.IsNull()) {
+    }
+    if (!field.IsNull()) {
       return Api::NewHandle(isolate, field.value());
-    } else {
-      return Api::NewError("%s: did not find top-level variable '%s'.",
-                           CURRENT_FUNC, field_name.ToCString());
     }
+    if (!ambiguity_error_msg.IsNull()) {
+      return Api::NewError("%s.",  ambiguity_error_msg.ToCString());
+    }
+    return Api::NewError("%s: did not find top-level variable '%s'.",
+                         CURRENT_FUNC, field_name.ToCString());
 
   } else if (obj.IsError()) {
       return container;
   } else {
     return Api::NewError(
         "%s expects argument 'container' to be an object, type, or library.",
         CURRENT_FUNC);
   }
 }
 
@@ skipping 107 matching lines @@
                                                           setter_name,
                                                           args,
                                                           args_descriptor));
     }
     return Api::NewHandle(isolate, DartEntry::InvokeFunction(setter, args));
 
   } else if (obj.IsLibrary()) {
     // To access a top-level we may need to use the Field or the
     // setter Function.  The setter function may either be in the
     // library or in the field's owner class, depending.
+    String& ambiguity_error_msg = String::Handle(isolate);
     const Library& lib = Library::Cast(obj);
-    field = lib.LookupFieldAllowPrivate(field_name);
-    if (field.IsNull()) {
+    field = lib.LookupFieldAllowPrivate(field_name, &ambiguity_error_msg);
+    if (field.IsNull() && ambiguity_error_msg.IsNull()) {
       const String& setter_name =
           String::Handle(isolate, Field::SetterName(field_name));
-      setter ^= lib.LookupFunctionAllowPrivate(setter_name);
+      setter ^= lib.LookupFunctionAllowPrivate(setter_name,
+                                               &ambiguity_error_msg);
     }
 
     if (!setter.IsNull()) {
       // Invoke the setter and return the result.
       const int kNumArgs = 1;
       const Array& args = Array::Handle(isolate, Array::New(kNumArgs));
       args.SetAt(0, value_instance);
       const Object& result =
           Object::Handle(isolate, DartEntry::InvokeFunction(setter, args));
       if (result.IsError()) {
         return Api::NewHandle(isolate, result.raw());
-      } else {
-        return Api::Success();
       }
-    } else if (!field.IsNull()) {
+      return Api::Success();
+    }
+    if (!field.IsNull()) {
       if (field.is_final()) {
         return Api::NewError("%s: cannot set final top-level variable '%s'.",
                              CURRENT_FUNC, field_name.ToCString());
-      } else {
-        field.set_value(value_instance);
-        return Api::Success();
       }
-    } else {
-      return Api::NewError("%s: did not find top-level variable '%s'.",
-                           CURRENT_FUNC, field_name.ToCString());
+      field.set_value(value_instance);
+      return Api::Success();
     }
+    if (!ambiguity_error_msg.IsNull()) {
+      return Api::NewError("%s.", ambiguity_error_msg.ToCString());
+    }
+    return Api::NewError("%s: did not find top-level variable '%s'.",
+                         CURRENT_FUNC, field_name.ToCString());
 
   } else if (obj.IsError()) {
-      return container;
-  } else {
-    return Api::NewError(
-        "%s expects argument 'container' to be an object, type, or library.",
-        CURRENT_FUNC);
+    return container;
   }
+  return Api::NewError(
+      "%s expects argument 'container' to be an object, type, or library.",
+      CURRENT_FUNC);
 }
 
 
 // --- Exceptions ----
 
 DART_EXPORT Dart_Handle Dart_ThrowException(Dart_Handle exception) {
   Isolate* isolate = Isolate::Current();
   CHECK_ISOLATE(isolate);
   CHECK_CALLBACK_STATE(isolate);
   {
@@ skipping 326 matching lines @@
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   const Library& lib = Api::UnwrapLibraryHandle(isolate, library);
   if (lib.IsNull()) {
     RETURN_TYPE_ERROR(isolate, library, Library);
   }
   const String& cls_name = Api::UnwrapStringHandle(isolate, class_name);
   if (cls_name.IsNull()) {
     RETURN_TYPE_ERROR(isolate, class_name, String);
   }
-  const Class& cls =
-      Class::Handle(isolate, lib.LookupClassAllowPrivate(cls_name));
+  String& ambiguity_error_msg = String::Handle(isolate);
+  const Class& cls = Class::Handle(
+      isolate, lib.LookupClassAllowPrivate(cls_name, &ambiguity_error_msg));
   if (cls.IsNull()) {
+    if (!ambiguity_error_msg.IsNull()) {
+      return Api::NewError("%s.", ambiguity_error_msg.ToCString());
+    }
     // TODO(turnidge): Return null or error in this case?
     const String& lib_name = String::Handle(isolate, lib.name());
     return Api::NewError("Class '%s' not found in library '%s'.",
                          cls_name.ToCString(), lib_name.ToCString());
   }
   return Api::NewHandle(isolate, cls.raw());
 }
 
 
 DART_EXPORT Dart_Handle Dart_GetType(Dart_Handle library,
@@ skipping 10 matching lines @@
   }
   const String& name_str = Api::UnwrapStringHandle(isolate, class_name);
   if (name_str.IsNull()) {
     RETURN_TYPE_ERROR(isolate, class_name, String);
   }
   // Ensure all classes are finalized.
   Dart_Handle state = Api::CheckIsolateState(isolate);
   if (::Dart_IsError(state)) {
     return state;
   }
+  String& ambiguity_error_msg = String::Handle(isolate);
   const Class& cls =
-      Class::Handle(isolate, lib.LookupClassAllowPrivate(name_str));
+      Class::Handle(isolate, lib.LookupClassAllowPrivate(name_str,
+                                                         &ambiguity_error_msg));
   if (cls.IsNull()) {
+    if (!ambiguity_error_msg.IsNull()) {
+      return Api::NewError("%s.", ambiguity_error_msg.ToCString());
+    }
     const String& lib_name = String::Handle(isolate, lib.name());
     return Api::NewError("Type '%s' not found in library '%s'.",
                          name_str.ToCString(), lib_name.ToCString());
   }
   if (cls.NumTypeArguments() == 0) {
     if (number_of_type_arguments != 0) {
       return Api::NewError("Invalid number of type arguments specified, "
                            "got %"Pd" expected 0", number_of_type_arguments);
     }
     return Api::NewHandle(isolate, Type::NewNonParameterizedType(cls));
@@ skipping 262 matching lines @@
   }
   {
     NoGCScope no_gc;
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success();
 }
 
 }  // namespace dart