From 70dc2fe968e353abe11b5bad86914e43e5ccedef Mon Sep 17 00:00:00 2001 From: "rossberg@chromium.org" Date: Mon, 24 Oct 2011 15:56:18 +0000 Subject: [PATCH] Implement for-in loop for proxies. Fix related corner case for Object.keys. Remove obsolete GET_KEYS builtin. R=ricow@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/8256015 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9760 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/api.cc | 8 +- src/arm/full-codegen-arm.cc | 28 ++++-- src/bootstrapper.cc | 1 + src/builtins.h | 1 - src/contexts.h | 2 + src/handles.cc | 25 ++++-- src/handles.h | 11 +-- src/ia32/full-codegen-ia32.cc | 30 +++++-- src/objects.cc | 7 +- src/objects.h | 6 +- src/proxy.js | 25 +++++- src/runtime.cc | 42 ++++++--- src/runtime.js | 7 -- src/v8natives.js | 4 +- src/x64/full-codegen-x64.cc | 29 +++++-- test/mjsunit/harmony/proxies-for.js | 168 ++++++++++++++++++++++++++++++++++++ test/mjsunit/harmony/proxies.js | 56 ++++++------ 17 files changed, 365 insertions(+), 85 deletions(-) create mode 100644 test/mjsunit/harmony/proxies-for.js diff --git a/src/api.cc b/src/api.cc index 8540dfd..ac4f07f 100644 --- a/src/api.cc +++ b/src/api.cc @@ -2874,8 +2874,10 @@ Local v8::Object::GetPropertyNames() { ENTER_V8(isolate); i::HandleScope scope(isolate); i::Handle self = Utils::OpenHandle(this); + bool threw = false; i::Handle value = - i::GetKeysInFixedArrayFor(self, i::INCLUDE_PROTOS); + i::GetKeysInFixedArrayFor(self, i::INCLUDE_PROTOS, &threw); + if (threw) return Local(); // Because we use caching to speed up enumeration it is important // to never change the result of the basic enumeration function so // we clone the result. @@ -2893,8 +2895,10 @@ Local v8::Object::GetOwnPropertyNames() { ENTER_V8(isolate); i::HandleScope scope(isolate); i::Handle self = Utils::OpenHandle(this); + bool threw = false; i::Handle value = - i::GetKeysInFixedArrayFor(self, i::LOCAL_ONLY); + i::GetKeysInFixedArrayFor(self, i::LOCAL_ONLY, &threw); + if (threw) return Local(); // Because we use caching to speed up enumeration it is important // to never change the result of the basic enumeration function so // we clone the result. diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 4ab158f..43e5c6a 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -929,11 +929,17 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { __ bind(&done_convert); __ push(r0); + // Check for proxies. + Label call_runtime; + STATIC_ASSERT(FIRST_JS_PROXY_TYPE == FIRST_SPEC_OBJECT_TYPE); + __ CompareObjectType(r0, r1, r1, LAST_JS_PROXY_TYPE); + __ b(le, &call_runtime); + // Check cache validity in generated code. This is a fast case for // the JSObject::IsSimpleEnum cache validity checks. If we cannot // guarantee cache validity, call the runtime system to check cache // validity or get the property names in a fixed array. - Label next, call_runtime; + Label next; // Preload a couple of values used in the loop. Register empty_fixed_array_value = r6; __ LoadRoot(empty_fixed_array_value, Heap::kEmptyFixedArrayRootIndex); @@ -1012,9 +1018,16 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { __ jmp(&loop); // We got a fixed array in register r0. Iterate through that. + Label non_proxy; __ bind(&fixed_array); - __ mov(r1, Operand(Smi::FromInt(0))); // Map (0) - force slow check. - __ Push(r1, r0); + __ mov(r1, Operand(Smi::FromInt(1))); // Smi indicates slow check + __ ldr(r2, MemOperand(sp, 0 * kPointerSize)); // Get enumerated object + STATIC_ASSERT(FIRST_JS_PROXY_TYPE == FIRST_SPEC_OBJECT_TYPE); + __ CompareObjectType(r2, r3, r3, LAST_JS_PROXY_TYPE); + __ b(gt, &non_proxy); + __ mov(r1, Operand(Smi::FromInt(0))); // Zero indicates proxy + __ bind(&non_proxy); + __ Push(r1, r0); // Smi and array __ ldr(r1, FieldMemOperand(r0, FixedArray::kLengthOffset)); __ mov(r0, Operand(Smi::FromInt(0))); __ Push(r1, r0); // Fixed array length (as smi) and initial index. @@ -1031,18 +1044,23 @@ void FullCodeGenerator::VisitForInStatement(ForInStatement* stmt) { __ add(r2, r2, Operand(FixedArray::kHeaderSize - kHeapObjectTag)); __ ldr(r3, MemOperand(r2, r0, LSL, kPointerSizeLog2 - kSmiTagSize)); - // Get the expected map from the stack or a zero map in the + // Get the expected map from the stack or a smi in the // permanent slow case into register r2. __ ldr(r2, MemOperand(sp, 3 * kPointerSize)); // Check if the expected map still matches that of the enumerable. - // If not, we have to filter the key. + // If not, we may have to filter the key. Label update_each; __ ldr(r1, MemOperand(sp, 4 * kPointerSize)); __ ldr(r4, FieldMemOperand(r1, HeapObject::kMapOffset)); __ cmp(r4, Operand(r2)); __ b(eq, &update_each); + // For proxies, no filtering is done. + // TODO(rossberg): What if only a prototype is a proxy? Not specified yet. + __ cmp(r2, Operand(Smi::FromInt(0))); + __ b(eq, &update_each); + // Convert the entry to a string or (smi) 0 if it isn't a property // any more. If the property has been removed while iterating, we // just skip it. diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 921c1b6..0446e3b 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1376,6 +1376,7 @@ void Genesis::InstallExperimentalNativeFunctions() { INSTALL_NATIVE(JSFunction, "DerivedHasTrap", derived_has_trap); INSTALL_NATIVE(JSFunction, "DerivedGetTrap", derived_get_trap); INSTALL_NATIVE(JSFunction, "DerivedSetTrap", derived_set_trap); + INSTALL_NATIVE(JSFunction, "ProxyEnumerate", proxy_enumerate); } } diff --git a/src/builtins.h b/src/builtins.h index 1d4c24c..24059e7 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -238,7 +238,6 @@ enum BuiltinExtraArguments { V(DELETE, 2) \ V(IN, 1) \ V(INSTANCE_OF, 1) \ - V(GET_KEYS, 0) \ V(FILTER_KEY, 1) \ V(CALL_NON_FUNCTION, 0) \ V(CALL_NON_FUNCTION_AS_CONSTRUCTOR, 0) \ diff --git a/src/contexts.h b/src/contexts.h index db725c3..c30fe6e 100644 --- a/src/contexts.h +++ b/src/contexts.h @@ -139,6 +139,7 @@ enum BindingFlags { V(DERIVED_HAS_TRAP_INDEX, JSFunction, derived_has_trap) \ V(DERIVED_GET_TRAP_INDEX, JSFunction, derived_get_trap) \ V(DERIVED_SET_TRAP_INDEX, JSFunction, derived_set_trap) \ + V(PROXY_ENUMERATE, JSFunction, proxy_enumerate) \ V(RANDOM_SEED_INDEX, ByteArray, random_seed) // JSFunctions are pairs (context, function code), sometimes also called @@ -260,6 +261,7 @@ class Context: public FixedArray { DERIVED_HAS_TRAP_INDEX, DERIVED_GET_TRAP_INDEX, DERIVED_SET_TRAP_INDEX, + PROXY_ENUMERATE, RANDOM_SEED_INDEX, // Properties from here are treated as weak references by the full GC. diff --git a/src/handles.cc b/src/handles.cc index 700a334..7ea494c 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -691,7 +691,7 @@ void CustomArguments::IterateInstance(ObjectVisitor* v) { // Compute the property keys from the interceptor. -v8::Handle GetKeysForNamedInterceptor(Handle receiver, +v8::Handle GetKeysForNamedInterceptor(Handle receiver, Handle object) { Isolate* isolate = receiver->GetIsolate(); Handle interceptor(object->GetNamedInterceptor()); @@ -713,7 +713,7 @@ v8::Handle GetKeysForNamedInterceptor(Handle receiver, // Compute the element keys from the interceptor. -v8::Handle GetKeysForIndexedInterceptor(Handle receiver, +v8::Handle GetKeysForIndexedInterceptor(Handle receiver, Handle object) { Isolate* isolate = receiver->GetIsolate(); Handle interceptor(object->GetIndexedInterceptor()); @@ -744,8 +744,9 @@ static bool ContainsOnlyValidKeys(Handle array) { } -Handle GetKeysInFixedArrayFor(Handle object, - KeyCollectionType type) { +Handle GetKeysInFixedArrayFor(Handle object, + KeyCollectionType type, + bool* threw) { USE(ContainsOnlyValidKeys); Isolate* isolate = object->GetIsolate(); Handle content = isolate->factory()->empty_fixed_array(); @@ -760,6 +761,16 @@ Handle GetKeysInFixedArrayFor(Handle object, for (Handle p = object; *p != isolate->heap()->null_value(); p = Handle(p->GetPrototype(), isolate)) { + if (p->IsJSProxy()) { + Handle proxy(JSProxy::cast(*p), isolate); + Handle args[] = { proxy }; + Handle names = Execution::Call( + isolate->proxy_enumerate(), object, ARRAY_SIZE(args), args, threw); + if (*threw) return content; + content = AddKeysFromJSArray(content, Handle::cast(names)); + break; + } + Handle current(JSObject::cast(*p), isolate); // Check access rights if required. @@ -826,11 +837,11 @@ Handle GetKeysInFixedArrayFor(Handle object, } -Handle GetKeysFor(Handle object) { +Handle GetKeysFor(Handle object, bool* threw) { Isolate* isolate = object->GetIsolate(); isolate->counters()->for_in()->Increment(); - Handle elements = GetKeysInFixedArrayFor(object, - INCLUDE_PROTOS); + Handle elements = + GetKeysInFixedArrayFor(object, INCLUDE_PROTOS, threw); return isolate->factory()->NewJSArrayWithElements(elements); } diff --git a/src/handles.h b/src/handles.h index 358f1b5..8876de3 100644 --- a/src/handles.h +++ b/src/handles.h @@ -295,18 +295,19 @@ int GetScriptLineNumberSafe(Handle