Author: [EMAIL PROTECTED]
Date: Fri Oct 24 03:59:40 2008
New Revision: 588
Removed:
changes/[EMAIL PROTECTED]/improve_context_lookup/
Modified:
branches/bleeding_edge/src/contexts.cc
branches/bleeding_edge/src/contexts.h
branches/bleeding_edge/src/objects.cc
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/test/mjsunit/regress/regress-124.js
Log:
Improve code for looking up in context slots in runtime.cc and
use safe casting operations to slot access on contexts when
possible.
Modified: branches/bleeding_edge/src/contexts.cc
==============================================================================
--- branches/bleeding_edge/src/contexts.cc (original)
+++ branches/bleeding_edge/src/contexts.cc Fri Oct 24 03:59:40 2008
@@ -27,6 +27,7 @@
#include "v8.h"
+#include "bootstrapper.h"
#include "debug.h"
#include "scopeinfo.h"
@@ -99,19 +100,19 @@
}
// check extension/with object
- Handle<JSObject> context_ext(context->extension());
- if (*context_ext != NULL) {
+ if (context->has_extension()) {
+ Handle<JSObject> extension = Handle<JSObject>(context->extension());
if ((flags & FOLLOW_PROTOTYPE_CHAIN) == 0) {
- *attributes = context_ext->GetLocalPropertyAttribute(*name);
+ *attributes = extension->GetLocalPropertyAttribute(*name);
} else {
- *attributes = context_ext->GetPropertyAttribute(*name);
+ *attributes = extension->GetPropertyAttribute(*name);
}
if (*attributes != ABSENT) {
// property found
if (FLAG_trace_contexts) {
- PrintF("=> found property in context object %p\n", *context_ext);
+ PrintF("=> found property in context object %p\n", *extension);
}
- return context_ext;
+ return extension;
}
}
@@ -184,11 +185,10 @@
// proceed with enclosing context
if (context->IsGlobalContext()) {
follow_context_chain = false;
- } else if (context->previous() != NULL) {
- context = Handle<Context>(context->previous());
- } else {
- ASSERT(context->is_function_context());
+ } else if (context->is_function_context()) {
context =
Handle<Context>(Context::cast(context->closure()->context()));
+ } else {
+ context = Handle<Context>(context->previous());
}
} while (follow_context_chain);
@@ -196,8 +196,23 @@
if (FLAG_trace_contexts) {
PrintF("=> no property/slot found\n");
}
- return Handle<Object>(reinterpret_cast<JSObject*>(NULL));
+ return Handle<Object>::null();
}
+
+#ifdef DEBUG
+bool Context::IsBootstrappingOrContext(Object* object) {
+ // During bootstrapping we allow all objects to pass as
+ // contexts. This is necessary to fix circular dependencies.
+ return Bootstrapper::IsActive() || object->IsContext();
+}
+
+
+bool Context::IsBootstrappingOrGlobalObject(Object* object) {
+ // During bootstrapping we allow all objects to pass as global
+ // objects. This is necessary to fix circular dependencies.
+ return Bootstrapper::IsActive() || object->IsGlobalObject();
+}
+#endif
} } // namespace v8::internal
Modified: branches/bleeding_edge/src/contexts.h
==============================================================================
--- branches/bleeding_edge/src/contexts.h (original)
+++ branches/bleeding_edge/src/contexts.h Fri Oct 24 03:59:40 2008
@@ -220,23 +220,24 @@
JSFunction* closure() { return JSFunction::cast(get(CLOSURE_INDEX)); }
void set_closure(JSFunction* closure) { set(CLOSURE_INDEX, closure); }
- Context* fcontext() {
- return reinterpret_cast<Context*>(get(FCONTEXT_INDEX));
- }
+ Context* fcontext() { return Context::cast(get(FCONTEXT_INDEX)); }
void set_fcontext(Context* context) { set(FCONTEXT_INDEX, context); }
Context* previous() {
- return reinterpret_cast<Context*>(get(PREVIOUS_INDEX));
+ Object* result = unchecked_previous();
+ ASSERT(IsBootstrappingOrContext(result));
+ return reinterpret_cast<Context*>(result);
}
void set_previous(Context* context) { set(PREVIOUS_INDEX, context); }
- JSObject* extension() {
- return reinterpret_cast<JSObject*>(get(EXTENSION_INDEX));
- }
+ bool has_extension() { return unchecked_extension() != NULL; }
+ JSObject* extension() { return JSObject::cast(unchecked_extension()); }
void set_extension(JSObject* object) { set(EXTENSION_INDEX, object); }
GlobalObject* global() {
- return reinterpret_cast<GlobalObject*>(get(GLOBAL_INDEX));
+ Object* result = get(GLOBAL_INDEX);
+ ASSERT(IsBootstrappingOrGlobalObject(result));
+ return reinterpret_cast<GlobalObject*>(result);
}
void set_global(GlobalObject* global) { set(GLOBAL_INDEX, global); }
@@ -251,7 +252,7 @@
Context* global_context();
// Tells if this is a function context (as opposed to a 'with' context).
- bool is_function_context() { return previous() == NULL; }
+ bool is_function_context() { return unchecked_previous() == NULL; }
// Tells whether the global context is marked with out of memory.
bool has_out_of_memory() {
@@ -293,7 +294,7 @@
// and the name is the property name, and the property exists.
// attributes != ABSENT.
//
- // 4) index_ < 0 && result.deref() == NULL:
+ // 4) index_ < 0 && result.is_null():
// there was no context found with the corresponding property.
// attributes == ABSENT.
Handle<Object> Lookup(Handle<String> name, ContextLookupFlags flags,
@@ -303,6 +304,17 @@
static int SlotOffset(int index) {
return kHeaderSize + index * kPointerSize - kHeapObjectTag;
}
+
+ private:
+ // Unchecked access to the slots.
+ Object* unchecked_previous() { return get(PREVIOUS_INDEX); }
+ Object* unchecked_extension() { return get(EXTENSION_INDEX); }
+
+#ifdef DEBUG
+ // Bootstrapping-aware type checks.
+ static bool IsBootstrappingOrContext(Object* object);
+ static bool IsBootstrappingOrGlobalObject(Object* object);
+#endif
};
} } // namespace v8::internal
Modified: branches/bleeding_edge/src/objects.cc
==============================================================================
--- branches/bleeding_edge/src/objects.cc (original)
+++ branches/bleeding_edge/src/objects.cc Fri Oct 24 03:59:40 2008
@@ -2139,7 +2139,7 @@
}
// Check the context extension if any.
- if (context->extension() != NULL) {
+ if (context->has_extension()) {
return context->extension()->ReferencesObject(obj);
}
}
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Fri Oct 24 03:59:40 2008
@@ -504,7 +504,7 @@
// "declared" in the function context's extension context, or in the
// global context.
Handle<JSObject> context_ext;
- if (context->extension() != NULL) {
+ if (context->has_extension()) {
// The function context's extension context exists - use it.
context_ext = Handle<JSObject>(context->extension());
} else {
@@ -3347,7 +3347,7 @@
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes);
- if (index < 0 && *holder != NULL) {
+ if (index < 0 && !holder.is_null()) {
ASSERT(holder->IsJSObject());
return *holder;
}
@@ -3364,62 +3364,48 @@
// compiler to do the right thing.
//
// TODO(1236026): This is a non-portable hack that should be removed.
-typedef uint64_t ObjPair;
-ObjPair MakePair(Object* x, Object* y) {
+typedef uint64_t ObjectPair;
+static inline ObjectPair MakePair(Object* x, Object* y) {
return reinterpret_cast<uint32_t>(x) |
- (reinterpret_cast<ObjPair>(y) << 32);
+ (reinterpret_cast<ObjectPair>(y) << 32);
}
-static Object* Unhole(Object* x, PropertyAttributes attributes) {
+static inline Object* Unhole(Object* x, PropertyAttributes attributes) {
ASSERT(!x->IsTheHole() || (attributes & READ_ONLY) != 0);
USE(attributes);
return x->IsTheHole() ? Heap::undefined_value() : x;
}
-static Object* ComputeContextSlotReceiver(Object* holder) {
- // If the "property" we were looking for is a local variable or an
- // argument in a context, the receiver is the global object; see
- // ECMA-262, 3rd., 10.1.6 and 10.2.3.
- // Contexts and global objects are most common.
- if (holder->IsContext()) {
- return Context::cast(holder)->global()->global_receiver();
- }
- if (holder->IsGlobalObject()) {
- // If the holder is a global object, we have to be careful to wrap
- // it in its proxy if necessary.
- return GlobalObject::cast(holder)->global_receiver();
- }
-
+static JSObject* ComputeReceiverForNonGlobal(JSObject* holder) {
+ ASSERT(!holder->IsGlobalObject());
Context* top = Top::context();
- // TODO(125): Find a better - and faster way - of checking for
- // arguments and context extension objects. This kinda sucks.
+ // Get the context extension function.
JSFunction* context_extension_function =
top->global_context()->context_extension_function();
- JSObject* arguments_boilerplate =
- top->global_context()->arguments_boilerplate();
- JSFunction* arguments_function =
- JSFunction::cast(arguments_boilerplate->map()->constructor());
- // If the holder is an arguments object or a context extension then the
- // receiver is also the global object;
- Object* constructor = HeapObject::cast(holder)->map()->constructor();
- if (constructor == context_extension_function ||
- constructor == arguments_function) {
- return Top::context()->global()->global_receiver();
- }
-
- return holder;
+ // If the holder isn't a context extension object, we just return it
+ // as the receiver. This allows arguments objects to be used as
+ // receivers, but only if they are put in the context scope chain
+ // explicitly via a with-statement.
+ Object* constructor = holder->map()->constructor();
+ if (constructor != context_extension_function) return holder;
+ // Fall back to using the global object as the receiver if the
+ // property turns out to be a local variable allocated in a context
+ // extension object - introduced via eval.
+ return top->global()->global_receiver();
}
-static ObjPair LoadContextSlotHelper(Arguments args, bool throw_error) {
+static ObjectPair LoadContextSlotHelper(Arguments args, bool throw_error) {
HandleScope scope;
ASSERT(args.length() == 2);
- if (!args[0]->IsContext()) return MakePair(IllegalOperation(), NULL);
+ if (!args[0]->IsContext() || !args[1]->IsString()) {
+ return MakePair(IllegalOperation(), NULL);
+ }
Handle<Context> context = args.at<Context>(0);
- Handle<String> name(String::cast(args[1]));
+ Handle<String> name = args.at<String>(1);
int index;
PropertyAttributes attributes;
@@ -3427,29 +3413,31 @@
Handle<Object> holder =
context->Lookup(name, flags, &index, &attributes);
+ // If the index is non-negative, the slot has been found in a local
+ // variable or a parameter. Read it from the context object or the
+ // arguments object.
if (index >= 0) {
- Handle<Object> receiver =
- Handle<Object>(ComputeContextSlotReceiver(*holder));
- Handle<Object> value;
- if (holder->IsContext()) {
- value = Handle<Object>(Context::cast(*holder)->get(index));
- } else {
- // Arguments object.
- value = Handle<Object>(JSObject::cast(*holder)->GetElement(index));
- }
- return MakePair(Unhole(*value, attributes), *receiver);
- }
-
- if (*holder != NULL) {
- ASSERT(Handle<JSObject>::cast(holder)->HasProperty(*name));
- // Note: As of 5/29/2008, GetProperty does the "unholing" and so
- // this call here is redundant. We left it anyway, to be explicit;
- // also it's not clear why GetProperty should do the unholing in
- // the first place.
- return MakePair(
- Unhole(Handle<JSObject>::cast(holder)->GetProperty(*name),
- attributes),
- ComputeContextSlotReceiver(*holder));
+ // If the "property" we were looking for is a local variable or an
+ // argument in a context, the receiver is the global object; see
+ // ECMA-262, 3rd., 10.1.6 and 10.2.3.
+ JSObject* receiver = Top::context()->global()->global_receiver();
+ Object* value = (holder->IsContext())
+ ? Context::cast(*holder)->get(index)
+ : JSObject::cast(*holder)->GetElement(index);
+ return MakePair(Unhole(value, attributes), receiver);
+ }
+
+ // If the holder is found, we read the property from it.
+ if (!holder.is_null() && holder->IsJSObject()) {
+ JSObject* object = JSObject::cast(*holder);
+ ASSERT(object->HasProperty(*name));
+ JSObject* receiver = (object->IsGlobalObject())
+ ? GlobalObject::cast(object)->global_receiver()
+ : ComputeReceiverForNonGlobal(object);
+ // No need to unhole the value here. This is taken care of by the
+ // GetProperty function.
+ Object* value = object->GetProperty(*name);
+ return MakePair(value, receiver);
}
if (throw_error) {
@@ -3464,12 +3452,12 @@
}
-static ObjPair Runtime_LoadContextSlot(Arguments args) {
+static ObjectPair Runtime_LoadContextSlot(Arguments args) {
return LoadContextSlotHelper(args, true);
}
-static ObjPair Runtime_LoadContextSlotNoReferenceError(Arguments args) {
+static ObjectPair Runtime_LoadContextSlotNoReferenceError(Arguments args) {
return LoadContextSlotHelper(args, false);
}
@@ -3508,7 +3496,7 @@
// It is either in an JSObject extension context or it was not found.
Handle<JSObject> context_ext;
- if (*holder != NULL) {
+ if (!holder.is_null()) {
// The property exists in the extension context.
context_ext = Handle<JSObject>::cast(holder);
} else {
@@ -4508,7 +4496,7 @@
Handle<String> name = info.LocalName(i);
// Traverse the context chain to the function context as all local
// variables stored in the context will be on the function context.
- while (context->previous() != NULL) {
+ while (!context->is_function_context()) {
context = Handle<Context>(context->previous());
}
ASSERT(context->is_function_context());
@@ -5024,7 +5012,7 @@
}
// Finally copy any properties from the function context extension. This
will
// be variables introduced by eval.
- if (function_context->extension() != NULL &&
+ if (function_context->has_extension() &&
!function_context->IsGlobalContext()) {
Handle<JSObject> ext(JSObject::cast(function_context->extension()));
Handle<FixedArray> keys = GetKeysInFixedArrayFor(ext);
Modified: branches/bleeding_edge/test/mjsunit/regress/regress-124.js
==============================================================================
--- branches/bleeding_edge/test/mjsunit/regress/regress-124.js (original)
+++ branches/bleeding_edge/test/mjsunit/regress/regress-124.js Fri Oct 24
03:59:40 2008
@@ -49,6 +49,9 @@
// Receiver should be the arguments object here.
assertEquals("[object Object]", eval("arguments[0]()"));
+ with (arguments) {
+ assertEquals("[object Object]", toString());
+ }
}
F(Object.prototype.toString);
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
