[v8-dev] Use output parameter to distinguish error from absent result (issue 1190023002 by [email protected])

Wed, 17 Jun 2015 06:24:13 -0700 

Reviewers: Jakob,

Message:
ptal

Description:
Use output parameter to distinguish error from absent result
Otherwise we'd have to probe for pending exceptions.

I'll do the same to other interceptors in follow-up CLs

BUG=chromium:495949,v8:4137
LOG=n

Please review this at https://codereview.chromium.org/1190023002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+28, -15 lines):
  M src/ic/ic.cc
  M src/objects.h
  M src/objects.cc


Index: src/ic/ic.cc
diff --git a/src/ic/ic.cc b/src/ic/ic.cc
index d04fc583c44a909cc0d57c900a7da160c88236aa..7c14b5625beb49203ee114964068d6a8cb6e1bad 100644 
--- a/src/ic/ic.cc
+++ b/src/ic/ic.cc
@@ -2771,10 +2771,11 @@ RUNTIME_FUNCTION(LoadPropertyWithInterceptorOnly) {
args.at<JSObject>(NamedLoadHandlerCompiler::kInterceptorArgsHolderIndex); 
   HandleScope scope(isolate);
   LookupIterator it(receiver, name, holder, LookupIterator::OWN);
-  auto res = JSObject::GetPropertyWithInterceptor(&it);
-  RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
+  bool done;
   Handle<Object> result;
-  if (res.ToHandle(&result)) return *result;
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, result, JSObject::GetPropertyWithInterceptor(&it, &done));
+  if (done) return *result;
   return isolate->heap()->no_interceptor_result_sentinel();
 }

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index bbf582a7b5d6dbbd90b9b9bbf0cce169a7c49e78..74586db26361938f22f53a560ba5a0759e5d4d1c 100644 
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -137,10 +137,12 @@ MaybeHandle<Object> Object::GetProperty(LookupIterator* it) { 
         return JSProxy::GetPropertyWithHandler(
             it->GetHolder<JSProxy>(), it->GetReceiver(), it->GetName());
       case LookupIterator::INTERCEPTOR: {
-        MaybeHandle<Object> maybe_result =
-            JSObject::GetPropertyWithInterceptor(it);
-        if (!maybe_result.is_null()) return maybe_result;
-        if (it->isolate()->has_pending_exception()) return maybe_result;
+        bool done;
+        Handle<Object> result;
+        ASSIGN_RETURN_ON_EXCEPTION(
+            it->isolate(), result,
+            JSObject::GetPropertyWithInterceptor(it, &done), Object);
+        if (done) return result;
         break;
       }
       case LookupIterator::ACCESS_CHECK:
@@ -493,9 +495,11 @@ MaybeHandle<Object> JSObject::GetPropertyWithFailedAccessCheck( 
       return GetPropertyWithAccessor(it);
     }
     DCHECK_EQ(LookupIterator::INTERCEPTOR, it->state());
-    auto result = GetPropertyWithInterceptor(it);
-    if (it->isolate()->has_scheduled_exception()) break;
-    if (!result.is_null()) return result;
+    bool done;
+    Handle<Object> result;
+    ASSIGN_RETURN_ON_EXCEPTION(it->isolate(), result,
+ GetPropertyWithInterceptor(it, &done), Object); 
+    if (done) return result;
   }
   it->isolate()->ReportFailedAccessCheck(checked);
   RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(it->isolate(), Object);
@@ -13314,7 +13318,9 @@ InterceptorInfo* JSObject::GetIndexedInterceptor() {
 }
-MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it) { +MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it, 
+                                                         bool* done) {
+  *done = false;
   Isolate* isolate = it->isolate();
   // Make sure that the top context does not change when doing callbacks or
   // interceptor calls.
@@ -13322,7 +13328,9 @@ MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it) { 

   DCHECK_EQ(LookupIterator::INTERCEPTOR, it->state());
   Handle<InterceptorInfo> interceptor = it->GetInterceptor();
-  if (interceptor->getter()->IsUndefined()) return MaybeHandle<Object>();
+  if (interceptor->getter()->IsUndefined()) {
+    return isolate->factory()->undefined_value();
+  }

   Handle<JSObject> holder = it->GetHolder<JSObject>();
   v8::Handle<v8::Value> result;
@@ -13340,7 +13348,7 @@ MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it) { 
     Handle<Name> name = it->name();

     if (name->IsSymbol() && !interceptor->can_intercept_symbols()) {
-      return MaybeHandle<Object>();
+      return isolate->factory()->undefined_value();
     }

     v8::GenericNamedPropertyGetterCallback getter =
@@ -13352,9 +13360,10 @@ MaybeHandle<Object> JSObject::GetPropertyWithInterceptor(LookupIterator* it) { 
   }

   RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object);
-  if (result.IsEmpty()) return MaybeHandle<Object>();
+  if (result.IsEmpty()) return isolate->factory()->undefined_value();
   Handle<Object> result_internal = v8::Utils::OpenHandle(*result);
   result_internal->VerifyApiCallResultType();
+  *done = true;
   // Rebox handle before return
   return handle(*result_internal, isolate);
 }
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index e484e3d0563e04bc639b18f816f0c2773dea1f56..2ba840bef4f88f07e2590617ef328225e61b4ff4 100644 
--- a/src/objects.h
+++ b/src/objects.h
@@ -1941,8 +1941,11 @@ class JSObject: public JSReceiver {
       Handle<JSObject> object,
       Handle<AccessorInfo> info);
+ // The result must be checked first for exceptions. If there's no exception, + // the output parameter |done| indicates whether the interceptor has a result 
+  // or not.
   MUST_USE_RESULT static MaybeHandle<Object> GetPropertyWithInterceptor(
-      LookupIterator* it);
+      LookupIterator* it, bool* done);

   // Accessors for hidden properties object.
   //


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to