Handle function proxies as getters/setters.

R=kmillikin@chromium.org
BUG=v8:1543
TEST=

Review URL: http://codereview.chromium.org/7849021

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9407 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/builtins.cc

@@ -441,7 +441,7 @@ MUST_USE_RESULT static MaybeObject* CallJsBuiltin(
   for (int i = 0; i < n_args; i++) {
     argv[i] = args.at<Object>(i + 1).location();
   }
-  bool pending_exception = false;
+  bool pending_exception;
   Handle<Object> result = Execution::Call(function,
                                           args.receiver(),
                                           n_args,

src/debug.cc

@@ -773,7 +773,7 @@ bool Debug::CompileDebuggerScript(int index) {
 
   // Execute the shared function in the debugger context.
   Handle<Context> context = isolate->global_context();
-  bool caught_exception = false;
+  bool caught_exception;
   Handle<JSFunction> function =
       factory->NewFunctionFromSharedFunctionInfo(function_info, context);
 
@@ -1104,7 +1104,7 @@ bool Debug::CheckBreakPoint(Handle<Object> break_point_object) {
   Handle<Object> break_id = factory->NewNumberFromInt(Debug::break_id());
 
   // Call HandleBreakPointx.
-  bool caught_exception = false;
+  bool caught_exception;
   const int argc = 2;
   Object** argv[argc] = {
     break_id.location(),
@@ -2353,7 +2353,7 @@ void Debugger::OnAfterCompile(Handle<Script> script,
   Handle<JSValue> wrapper = GetScriptWrapper(script);
 
   // Call UpdateScriptBreakPoints expect no exceptions.
-  bool caught_exception = false;
+  bool caught_exception;
   const int argc = 1;
   Object** argv[argc] = { reinterpret_cast<Object**>(wrapper.location()) };
   Execution::TryCall(Handle<JSFunction>::cast(update_script_break_points),
@@ -2494,7 +2494,7 @@ void Debugger::CallJSEventCallback(v8::DebugEvent event,
                           exec_state.location(),
                           Handle<Object>::cast(event_data).location(),
                           event_listener_data_.location() };
-  bool caught_exception = false;
+  bool caught_exception;
   Execution::TryCall(fun, isolate_->global(), argc, argv, &caught_exception);
   // Silently ignore exceptions from debug event listeners.
 }

src/execution.cc

@@ -151,6 +151,8 @@ Handle<Object> Execution::Call(Handle<Object> callable,
                                Object*** args,
                                bool* pending_exception,
                                bool convert_receiver) {
+  *pending_exception = false;
+
   if (!callable->IsJSFunction()) {
     callable = TryGetFunctionDelegate(callable, pending_exception);
     if (*pending_exception) return callable;
@@ -195,6 +197,7 @@ Handle<Object> Execution::TryCall(Handle<JSFunction> func,
   v8::TryCatch catcher;
   catcher.SetVerbose(false);
   catcher.SetCaptureMessage(false);
+  *caught_exception = false;
 
   Handle<Object> result = Invoke(false, func, receiver, argc, args,
                                  caught_exception);
@@ -756,7 +759,7 @@ Handle<String> Execution::GetStackTraceLine(Handle<Object> recv,
                           Handle<Object>::cast(fun).location(),
                           pos.location(),
                           is_global.location() };
-  bool caught_exception = false;
+  bool caught_exception;
   Handle<Object> result =
       TryCall(isolate->get_stack_trace_line_fun(),
               isolate->js_builtins_object(), argc, args,

src/objects-inl.h

@@ -174,6 +174,13 @@ bool Object::IsSpecObject() {
 }
 
 
+bool Object::IsSpecFunction() {
+  if (!Object::IsHeapObject()) return false;
+  InstanceType type = HeapObject::cast(this)->map()->instance_type();
+  return type == JS_FUNCTION_TYPE || type == JS_FUNCTION_PROXY_TYPE;
+}
+
+
 bool Object::IsSymbol() {
   if (!this->IsHeapObject()) return false;
   uint32_t type = HeapObject::cast(this)->map()->instance_type();

src/objects.cc

@@ -205,8 +205,9 @@ MaybeObject* JSObject::GetPropertyWithCallback(Object* receiver,
   // __defineGetter__ callback
   if (structure->IsFixedArray()) {
     Object* getter = FixedArray::cast(structure)->get(kGetterIndex);
-    if (getter->IsJSFunction()) {
-      return GetPropertyWithDefinedGetter(receiver, JSFunction::cast(getter));
+    if (getter->IsSpecFunction()) {
+      // TODO(rossberg): nicer would be to cast to some JSCallable here...
+      return GetPropertyWithDefinedGetter(receiver, JSReceiver::cast(getter));
     }
     // Getter is not a function.
     return isolate->heap()->undefined_value();
@@ -261,17 +262,20 @@ bool JSProxy::HasElementWithHandler(uint32_t index) {
 
 
 MaybeObject* Object::GetPropertyWithDefinedGetter(Object* receiver,
-                                                  JSFunction* getter) {
+                                                  JSReceiver* getter) {
   HandleScope scope;
-  Handle<JSFunction> fun(JSFunction::cast(getter));
+  Handle<JSReceiver> fun(getter);
   Handle<Object> self(receiver);
 #ifdef ENABLE_DEBUGGER_SUPPORT
   Debug* debug = fun->GetHeap()->isolate()->debug();
   // Handle stepping into a getter if step into is active.
-  if (debug->StepInActive()) {
-    debug->HandleStepIn(fun, Handle<Object>::null(), 0, false);
+  // TODO(rossberg): should this apply to getters that are function proxies?
+  if (debug->StepInActive() && fun->IsJSFunction()) {
+    debug->HandleStepIn(
+        Handle<JSFunction>::cast(fun), Handle<Object>::null(), 0, false);
   }
 #endif
+
   bool has_pending_exception;
   Handle<Object> result =
       Execution::Call(fun, self, 0, NULL, &has_pending_exception);
@@ -1882,8 +1886,9 @@ MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
 
   if (structure->IsFixedArray()) {
     Object* setter = FixedArray::cast(structure)->get(kSetterIndex);
-    if (setter->IsJSFunction()) {
-     return SetPropertyWithDefinedSetter(JSFunction::cast(setter), value);
+    if (setter->IsSpecFunction()) {
+      // TODO(rossberg): nicer would be to cast to some JSCallable here...
+     return SetPropertyWithDefinedSetter(JSReceiver::cast(setter), value);
     } else {
       if (strict_mode == kNonStrictMode) {
         return value;
@@ -1902,17 +1907,19 @@ MaybeObject* JSObject::SetPropertyWithCallback(Object* structure,
 }
 
 
-MaybeObject* JSReceiver::SetPropertyWithDefinedSetter(JSFunction* setter,
+MaybeObject* JSReceiver::SetPropertyWithDefinedSetter(JSReceiver* setter,
                                                       Object* value) {
   Isolate* isolate = GetIsolate();
   Handle<Object> value_handle(value, isolate);
-  Handle<JSFunction> fun(JSFunction::cast(setter), isolate);
+  Handle<JSReceiver> fun(setter, isolate);
   Handle<JSReceiver> self(this, isolate);
 #ifdef ENABLE_DEBUGGER_SUPPORT
   Debug* debug = isolate->debug();
   // Handle stepping into a setter if step into is active.
-  if (debug->StepInActive()) {
-    debug->HandleStepIn(fun, Handle<Object>::null(), 0, false);
+  // TODO(rossberg): should this apply to getters that are function proxies?
+  if (debug->StepInActive() && fun->IsJSFunction()) {
+    debug->HandleStepIn(
+        Handle<JSFunction>::cast(fun), Handle<Object>::null(), 0, false);
   }
 #endif
   bool has_pending_exception;
@@ -2323,8 +2330,9 @@ MUST_USE_RESULT MaybeObject* JSProxy::SetPropertyWithHandlerIfDefiningSetter(
     ASSERT(!isolate->has_pending_exception());
     if (!setter->IsUndefined()) {
       // We have a setter -- invoke it.
+      // TODO(rossberg): nicer would be to cast to some JSCallable here...
       return proxy->SetPropertyWithDefinedSetter(
-          JSFunction::cast(*setter), *value);
+          JSReceiver::cast(*setter), *value);
     } else {
       Handle<String> get_name = isolate->factory()->LookupAsciiSymbol("get_");
       Handle<Object> getter(v8::internal::GetProperty(desc, get_name));
@@ -2482,7 +2490,7 @@ MUST_USE_RESULT Handle<Object> JSProxy::CallTrap(
   }
 
   Object*** argv = reinterpret_cast<Object***>(args);
-  bool threw = false;
+  bool threw;
   return Execution::Call(trap, handler, argc, argv, &threw);
 }
 
@@ -3977,7 +3985,7 @@ MaybeObject* JSObject::DefineAccessor(String* name,
                                       bool is_getter,
                                       Object* fun,
                                       PropertyAttributes attributes) {
-  ASSERT(fun->IsJSFunction() || fun->IsUndefined());
+  ASSERT(fun->IsSpecFunction() || fun->IsUndefined());
   Isolate* isolate = GetIsolate();
   // Check access rights if needed.
   if (IsAccessCheckNeeded() &&
@@ -8418,8 +8426,9 @@ MaybeObject* JSObject::GetElementWithCallback(Object* receiver,
   // __defineGetter__ callback
   if (structure->IsFixedArray()) {
     Object* getter = FixedArray::cast(structure)->get(kGetterIndex);
-    if (getter->IsJSFunction()) {
-      return GetPropertyWithDefinedGetter(receiver, JSFunction::cast(getter));
+    if (getter->IsSpecFunction()) {
+      // TODO(rossberg): nicer would be to cast to some JSCallable here...
+      return GetPropertyWithDefinedGetter(receiver, JSReceiver::cast(getter));
     }
     // Getter is not a function.
     return isolate->heap()->undefined_value();
@@ -8474,8 +8483,9 @@ MaybeObject* JSObject::SetElementWithCallback(Object* structure,
 
   if (structure->IsFixedArray()) {
     Handle<Object> setter(FixedArray::cast(structure)->get(kSetterIndex));
-    if (setter->IsJSFunction()) {
-     return SetPropertyWithDefinedSetter(JSFunction::cast(*setter), value);
+    if (setter->IsSpecFunction()) {
+      // TODO(rossberg): nicer would be to cast to some JSCallable here...
+      return SetPropertyWithDefinedSetter(JSReceiver::cast(*setter), value);
     } else {
       if (strict_mode == kNonStrictMode) {
         return value;

src/objects.h

@@ -863,6 +863,9 @@ class MaybeObject BASE_EMBEDDED {
   V(AccessCheckNeeded)                         \
   V(JSGlobalPropertyCell)                      \
 
+
+class JSReceiver;
+
 // Object is the abstract superclass for all classes in the
 // object hierarchy.
 // Object does not use any virtual functions to avoid the
@@ -887,6 +890,7 @@ class Object : public MaybeObject {
 #undef DECLARE_STRUCT_PREDICATE
 
   INLINE(bool IsSpecObject());
+  INLINE(bool IsSpecFunction());
 
   // Oddball testing.
   INLINE(bool IsUndefined());
@@ -936,7 +940,7 @@ class Object : public MaybeObject {
                                            String* key,
                                            PropertyAttributes* attributes);
   MUST_USE_RESULT MaybeObject* GetPropertyWithDefinedGetter(Object* receiver,
-                                                            JSFunction* getter);
+                                                            JSReceiver* getter);
 
   inline MaybeObject* GetElement(uint32_t index);
   // For use when we know that no exception can be thrown.
@@ -1356,7 +1360,7 @@ class JSReceiver: public HeapObject {
                                            Object* value,
                                            PropertyAttributes attributes,
                                            StrictModeFlag strict_mode);
-  MUST_USE_RESULT MaybeObject* SetPropertyWithDefinedSetter(JSFunction* setter,
+  MUST_USE_RESULT MaybeObject* SetPropertyWithDefinedSetter(JSReceiver* setter,
                                                             Object* value);
 
   MUST_USE_RESULT MaybeObject* DeleteProperty(String* name, DeleteMode mode);

src/runtime.cc

@@ -4206,7 +4206,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DefineOrRedefineAccessorProperty) {
   CONVERT_CHECKED(String, name, args[1]);
   CONVERT_CHECKED(Smi, flag_setter, args[2]);
   Object* fun = args[3];
-  RUNTIME_ASSERT(fun->IsJSFunction() || fun->IsUndefined());
+  RUNTIME_ASSERT(fun->IsSpecFunction() || fun->IsUndefined());
   CONVERT_CHECKED(Smi, flag_attr, args[4]);
   int unchecked = flag_attr->value();
   RUNTIME_ASSERT((unchecked & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0);
@@ -8422,7 +8422,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_Apply) {
      argv[i] = Handle<Object>(object);
   }
 
-  bool threw = false;
+  bool threw;
   Handle<JSReceiver> hfun(fun);
   Handle<Object> hreceiver(receiver);
   Handle<Object> result = Execution::Call(
@@ -12957,7 +12957,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetFromCache) {
     Handle<Object> receiver(isolate->global_context()->global());
     // This handle is nor shared, nor used later, so it's safe.
     Object** argv[] = { key_handle.location() };
-    bool pending_exception = false;
+    bool pending_exception;
     value = Execution::Call(factory,
                             receiver,
                             1,

test/mjsunit/harmony/proxies.js

@@ -2019,7 +2019,7 @@ function CreateFrozen(handler, callTrap, constructTrap) {
 
 function TestCall(isStrict, callTrap) {
   assertEquals(42, callTrap(5, 37))
-  // TODO(rossberg): unrelated bug: this does not succeed for optimized code.
+  // TODO(rossberg): unrelated bug: this does not succeed for optimized code:
   // assertEquals(isStrict ? undefined : global_object, receiver)
 
   var f = Proxy.createFunction({}, callTrap)
@@ -2236,3 +2236,120 @@ function TestConstructThrow2(f) {
 TestConstructThrow(function() { throw "myexn" })
 TestConstructThrow(Proxy.createFunction({}, function() { throw "myexn" }))
 TestConstructThrow(CreateFrozen({}, function() { throw "myexn" }))
+
+
+
+// Getters and setters.
+
+var value
+var receiver
+
+function TestAccessorCall(getterCallTrap, setterCallTrap) {
+  var handler = {fix: function() { return {} }}
+  var pgetter = Proxy.createFunction(handler, getterCallTrap)
+  var psetter = Proxy.createFunction(handler, setterCallTrap)
+
+  var o = {}
+  var oo = Object.create(o)
+  Object.defineProperty(o, "a", {get: pgetter, set: psetter})
+  Object.defineProperty(o, "b", {get: pgetter})
+  Object.defineProperty(o, "c", {set: psetter})
+  Object.defineProperty(o, "3", {get: pgetter, set: psetter})
+  Object.defineProperty(oo, "a", {value: 43})
+
+  receiver = ""
+  assertEquals(42, o.a)
+  assertSame(o, receiver)
+  receiver = ""
+  assertEquals(42, o.b)
+  assertSame(o, receiver)
+  receiver = ""
+  assertEquals(undefined, o.c)
+  assertEquals("", receiver)
+  receiver = ""
+  assertEquals(42, o["a"])
+  assertSame(o, receiver)
+  receiver = ""
+  assertEquals(42, o[3])
+  assertSame(o, receiver)
+
+  receiver = ""
+  assertEquals(43, oo.a)
+  assertEquals("", receiver)
+  receiver = ""
+  assertEquals(42, oo.b)
+  assertSame(o, receiver)
+  receiver = ""
+  assertEquals(undefined, oo.c)
+  assertEquals("", receiver)
+  receiver = ""
+  assertEquals(43, oo["a"])
+  assertEquals("", receiver)
+  receiver = ""
+  assertEquals(42, oo[3])
+  assertSame(o, receiver)
+
+  receiver = ""
+  assertEquals(50, o.a = 50)
+  assertSame(o, receiver)
+  assertEquals(50, value)
+  receiver = ""
+  assertEquals(51, o.b = 51)
+  assertEquals("", receiver)
+  assertEquals(50, value)  // no setter
+  assertThrows(function() { "use strict"; o.b = 51 }, TypeError)
+  receiver = ""
+  assertEquals(52, o.c = 52)
+  assertSame(o, receiver)
+  assertEquals(52, value)
+  receiver = ""
+  assertEquals(53, o["a"] = 53)
+  assertSame(o, receiver)
+  assertEquals(53, value)
+  receiver = ""
+  assertEquals(54, o[3] = 54)
+  assertSame(o, receiver)
+  assertEquals(54, value)
+
+  value = 0
+  receiver = ""
+  assertEquals(60, oo.a = 60)
+  assertEquals("", receiver)
+  assertEquals(0, value)  // oo has own 'a'
+  assertEquals(61, oo.b = 61)
+  assertSame("", receiver)
+  assertEquals(0, value)  // no setter
+  assertThrows(function() { "use strict"; oo.b = 61 }, TypeError)
+  receiver = ""
+  assertEquals(62, oo.c = 62)
+  assertSame(oo, receiver)
+  assertEquals(62, value)
+  receiver = ""
+  assertEquals(63, oo["c"] = 63)
+  assertSame(oo, receiver)
+  assertEquals(63, value)
+  receiver = ""
+  assertEquals(64, oo[3] = 64)
+  assertSame(oo, receiver)
+  assertEquals(64, value)
+}
+
+TestAccessorCall(
+  function() { receiver = this; return 42 },
+  function(x) { receiver = this; value = x }
+)
+
+TestAccessorCall(
+  function() { "use strict"; receiver = this; return 42 },
+  function(x) { "use strict"; receiver = this; value = x }
+)
+
+TestAccessorCall(
+  Proxy.createFunction({}, function() { receiver = this; return 42 }),
+  Proxy.createFunction({}, function(x) { receiver = this; value = x })
+)
+
+TestAccessorCall(
+  CreateFrozen({}, function() { receiver = this; return 42 }),
+  CreateFrozen({}, function(x) { receiver = this; value = x })
+)