Cleanup api for isolate state checking.

runtime/vm/dart_api_impl.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 "include/dart_api.h"
 
 #include "vm/bigint_operations.h"
 #include "vm/class_finalizer.h"
 #include "vm/compiler.h"
 #include "vm/dart.h"
@@ skipping 61 matching lines @@
     intptr_t len = (length);                                                   \
     intptr_t max = (max_elements);                                             \
     if (len < 0 || len > max) {                                                \
       return Api::NewError(                                                    \
           "%s expects argument '%s' to be in the range [0..%"Pd"].",           \
           CURRENT_FUNC, #length, max);                                         \
     }                                                                          \
   } while (0)
 
 
-// Return error if isolate is in an inconsistent state.
-// Return NULL when no error condition exists.
-//
-// TODO(turnidge): Make this function return an error handle directly
-// rather than returning an error string.  The current behavior can
-// cause compilation errors to appear to be api errors.
-const char* CheckIsolateState(Isolate* isolate) {
-  if (ClassFinalizer::FinalizePendingClasses() &&
-      isolate->object_store()->PreallocateObjects()) {
-    // Success.
-    return NULL;
-  }
-  // Make a copy of the error message as the original message string
-  // may get deallocated when we return back from the Dart API call.
-  const Error& err = Error::Handle(isolate->object_store()->sticky_error());
-  const char* errmsg = err.ToErrorCString();
-  intptr_t errlen = strlen(errmsg) + 1;
-  char* msg = Api::TopScope(isolate)->zone()->Alloc<char>(errlen);
-  OS::SNPrint(msg, errlen, "%s", errmsg);
-  return msg;
-}
-
-
 void SetupErrorResult(Isolate* isolate, Dart_Handle* handle) {
   *handle = Api::NewHandle(
       isolate, Isolate::Current()->object_store()->sticky_error());
 }
 
 
 Dart_Handle Api::NewHandle(Isolate* isolate, RawObject* raw) {
   LocalHandles* local_handles = Api::TopScope(isolate)->local_handles();
   ASSERT(local_handles != NULL);
   LocalHandle* ref = local_handles->AllocateHandle();
@@ skipping 54 matching lines @@
 
 
 FinalizablePersistentHandle* Api::UnwrapAsPrologueWeakPersistentHandle(
     const ApiState& state,
     Dart_Handle object) {
   ASSERT(state.IsValidPrologueWeakPersistentHandle(object));
   return reinterpret_cast<FinalizablePersistentHandle*>(object);
 }
 
 
+Dart_Handle Api::CheckIsolateState(Isolate* isolate) {
+  if (ClassFinalizer::FinalizePendingClasses() &&
+      isolate->object_store()->PreallocateObjects()) {
+    return Api::Success(isolate);
+  }
+  const Object& obj = Object::Handle(isolate->object_store()->sticky_error());
+  if (obj.IsError()) {

[siva, 2013/02/01 22:56:26]

Shouldn't this always be an Error, in what cases can it not be an error.

Maybe we just assert it is an Error here.

[regis, 2013/02/01 23:43:16]

Done.

+    return Api::NewHandle(isolate, obj.raw());
+  }
+  ASSERT(obj.IsNull());
+  return Api::Success(isolate);
+}
+
+
 Dart_Isolate Api::CastIsolate(Isolate* isolate) {
   return reinterpret_cast<Dart_Isolate>(isolate);
 }
 
 
 Dart_Handle Api::Success(Isolate* isolate) {
   ASSERT(isolate != NULL);
   ApiState* state = isolate->api_state();
   ASSERT(state != NULL);
   PersistentHandle* true_handle = state->True();
@@ skipping 709 matching lines @@
                                             intptr_t* size) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   TIMERSCOPE(time_creating_snapshot);
   if (buffer == NULL) {
     RETURN_NULL_ERROR(buffer);
   }
   if (size == NULL) {
     RETURN_NULL_ERROR(size);
   }
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle state = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(state)) {
+    return state;
   }
   // Since this is only a snapshot the root library should not be set.
   isolate->object_store()->set_root_library(Library::Handle(isolate));
   FullSnapshotWriter writer(buffer, ApiReallocate);
   writer.WriteFullSnapshot();
   *size = writer.BytesWritten();
   return Api::Success(isolate);
 }
 
 
 DART_EXPORT Dart_Handle Dart_CreateScriptSnapshot(uint8_t** buffer,
                                                   intptr_t* size) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   TIMERSCOPE(time_creating_snapshot);
   if (buffer == NULL) {
     RETURN_NULL_ERROR(buffer);
   }
   if (size == NULL) {
     RETURN_NULL_ERROR(size);
   }
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle state = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(state)) {
+    return state;
   }
   Library& library =
       Library::Handle(isolate, isolate->object_store()->root_library());
   if (library.IsNull()) {
     return
         Api::NewError("%s expects the isolate to have a script loaded in it.",
                       CURRENT_FUNC);
   }
   ScriptSnapshotWriter writer(buffer, ApiReallocate);
   writer.WriteScriptSnapshot(library);
@@ skipping 303 matching lines @@
   }
   const Object& obj = Object::Handle(isolate, Api::UnwrapHandle(object));
   if (obj.IsError()) {
     return object;
   } else if (!obj.IsNull() && !obj.IsInstance()) {
     return Api::NewError(
         "%s expects argument 'object' to be an instance of Object.",
         CURRENT_FUNC);
   }
   // Finalize all classes.
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle state = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(state)) {
+    return state;
   }
   if (obj.IsInstance()) {
     CHECK_CALLBACK_STATE(isolate);
     const Type& type = Type::Handle(isolate,
                                     Type::NewNonParameterizedType(cls));
     Error& malformed_type_error = Error::Handle(isolate);
     *value = Instance::Cast(obj).IsInstanceOf(type,
                                               TypeArguments::Handle(isolate),
                                               &malformed_type_error);
     ASSERT(malformed_type_error.IsNull());  // Type was created from a class.
@@ skipping 1252 matching lines @@
 DART_EXPORT Dart_Handle Dart_ClassGetInterfaceAt(Dart_Handle clazz,
                                                  intptr_t index) {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
   const Class& cls = Api::UnwrapClassHandle(isolate, clazz);
   if (cls.IsNull()) {
     RETURN_TYPE_ERROR(isolate, clazz, Class);
   }
 
   // Finalize all classes.
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle state = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(state)) {
+    return state;
   }
 
   const Array& interface_types = Array::Handle(isolate, cls.interfaces());
   if (index < 0 || index >= interface_types.Length()) {
     return Api::NewError("%s: argument 'index' out of bounds.", CURRENT_FUNC);
   }
   Type& interface_type = Type::Handle(isolate);
   interface_type ^= interface_types.At(index);
   if (interface_type.HasResolvedTypeClass()) {
     return Api::NewHandle(isolate, interface_type.type_class());
@@ skipping 743 matching lines @@
   String& dot_name = String::Handle(isolate);
   const Object& name_obj =
       Object::Handle(isolate, Api::UnwrapHandle(constructor_name));
   if (name_obj.IsNull()) {
     dot_name = Symbols::Dot().raw();
   } else if (name_obj.IsString()) {
     dot_name = String::Concat(Symbols::Dot(), String::Cast(name_obj));
   } else {
     RETURN_TYPE_ERROR(isolate, constructor_name, String);
   }
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle state = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(state)) {
+    return state;
   }
 
   // Resolve the constructor.
   result = ResolveConstructor(
       "Dart_New", cls, base_constructor_name, dot_name, number_of_arguments);
   if (result.IsError()) {
     return Api::NewHandle(isolate, result.raw());
   }
   // TODO(turnidge): Support redirecting factories.
   ASSERT(result.IsFunction());
@@ skipping 107 matching lines @@
       return Api::NewError("%s: did not find instance method '%s.%s'.",
                            CURRENT_FUNC,
                            cls_name.ToCString(),
                            function_name.ToCString());
     }
     args.SetAt(0, instance);
     return Api::NewHandle(isolate, DartEntry::InvokeDynamic(function, args));
 
   } else if (obj.IsClass()) {
     // Finalize all classes.
-    const char* msg = CheckIsolateState(isolate);
-    if (msg != NULL) {
-      return Api::NewError("%s", msg);
+    Dart_Handle state = Api::CheckIsolateState(isolate);
+    if (Dart_IsError(state)) {
+      return state;
     }
 
     const Class& cls = Class::Cast(obj);
     const Function& function = Function::Handle(
         isolate,
         Resolver::ResolveStatic(cls,
                                 function_name,
                                 number_of_arguments,
                                 Object::empty_array(),
                                 Resolver::kIsQualified));
@@ skipping 14 matching lines @@
     // When calling functions in the dart:builtin library do not finalize as it
     // should have been prefinalized.
     Library& builtin =
         Library::Handle(isolate, isolate->object_store()->builtin_library());
     if (builtin.raw() == lib.raw()) {
       finalize_classes = false;
     }
 
     // Finalize all classes if needed.
     if (finalize_classes) {
-      const char* msg = CheckIsolateState(isolate);
-      if (msg != NULL) {
-        return Api::NewError("%s", msg);
+      Dart_Handle state = Api::CheckIsolateState(isolate);
+      if (Dart_IsError(state)) {
+        return state;
       }
     }
 
     Function& function = Function::Handle(isolate);
     function = lib.LookupFunctionAllowPrivate(function_name);
     if (function.IsNull()) {
       return Api::NewError("%s: did not find top-level function '%s'.",
                            CURRENT_FUNC,
                            function_name.ToCString());
     }
@@ skipping 69 matching lines @@
     }
 
     // Invoke the getter and return the result.
     const int kNumArgs = 1;
     const Array& args = Array::Handle(isolate, Array::New(kNumArgs));
     args.SetAt(0, instance);
     return Api::NewHandle(isolate, DartEntry::InvokeDynamic(getter, args));
 
   } else if (obj.IsClass()) {
     // Finalize all classes.
-    const char* msg = CheckIsolateState(isolate);
-    if (msg != NULL) {
-      return Api::NewError("%s", msg);
+    Dart_Handle state = Api::CheckIsolateState(isolate);
+    if (Dart_IsError(state)) {
+      return state;
     }
     // To access a static field we may need to use the Field or the
     // getter Function.
     const Class& cls = Class::Cast(obj);
     field = cls.LookupStaticField(field_name);
     if (field.IsNull() || FieldIsUninitialized(isolate, field)) {
       const String& getter_name =
           String::Handle(isolate, Field::GetterName(field_name));
       getter = cls.LookupStaticFunctionAllowPrivate(getter_name);
     }
@@ skipping 562 matching lines @@
     *result = Api::Success(isolate);
   } else {
     *result = Api::NewHandle(isolate, error.raw());
   }
 }
 
 
 DART_EXPORT Dart_Handle Dart_CompileAll() {
   Isolate* isolate = Isolate::Current();
   DARTSCOPE(isolate);
-  Dart_Handle result;
-  const char* msg = CheckIsolateState(isolate);
-  if (msg != NULL) {
-    return Api::NewError("%s", msg);
+  Dart_Handle result = Api::CheckIsolateState(isolate);
+  if (Dart_IsError(result)) {
+    return result;
   }
   CHECK_CALLBACK_STATE(isolate);
   CompileAll(isolate, &result);
   return result;
 }
 
 
 DART_EXPORT bool Dart_IsLibrary(Dart_Handle object) {
   return Api::ClassId(object) == kLibraryCid;
 }
@@ skipping 129 matching lines @@
   Dart_Handle result;
   CompileSource(isolate, library, script, &result);
   // Propagate the error out right now.
   if (::Dart_IsError(result)) {
     return result;
   }
 
   // If this is the dart:builtin library, register it with the VM.
   if (url_str.Equals("dart:builtin")) {
     isolate->object_store()->set_builtin_library(library);
-    const char* msg = CheckIsolateState(isolate);
-    if (msg != NULL) {
-      return Api::NewError("%s", msg);
+    Dart_Handle state = Api::CheckIsolateState(isolate);
+    if (Dart_IsError(state)) {
+      return state;
     }
   }
   return result;
 }
 
 
 DART_EXPORT Dart_Handle Dart_LibraryImportLibrary(Dart_Handle library,
                                                   Dart_Handle import,
                                                   Dart_Handle prefix) {
   Isolate* isolate = Isolate::Current();
@@ skipping 154 matching lines @@
   }
   {
     NoGCScope no_gc;
     RawObject* raw_obj = obj.raw();
     isolate->heap()->SetPeer(raw_obj, peer);
   }
   return Api::Success(isolate);
 }
 
 }  // namespace dart