Reviewers: Kevin Millikin,
Description:
Handle CALLBACKS correctly in IsProperty functions.
With transitions in AccessorPairs, it is not enough to look at the
PropertyType
alone to decide whether we look at a property or not: For objects with
JavaScript accessors, we have to look into the AccessorPair itself and see
if
one of its 2 parts is actually a JavaScript accessor. Therefore, a predicate
with a PropertyType argument alone doesn't make sense anymore, we might
need the
associated value, too.
Things are complicated by the fact that the holder in a LookupResult can be
NULL, so we must be careful to retrieve its value only when it is really
needed. To achieve the needed call-by-name semantics, a new Entry is
introduced,
which is basically a closure over a DescriptorArray and an index into this
array
(C++0x to the rescue!). GCC is clever enough to inline this class, so we
pay no
runtime penalty for this abstraction.
It's all a bit ugly, but this is caused by the current structure of
Descriptor,
DescriptorArray and LookupResult: Things would be much easier if
DescriptorArray
were, well, an array of Descriptors, and LookupResult were a 'Maybe
Descriptor'
(in Haskell-terms).
Please review this at http://codereview.chromium.org/9466047/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/objects-inl.h
M src/objects.h
M src/property-details.h
M src/property.h
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
bc6217ba5ebe66ae613719d5c67c06a91952c363..176be9b6ed903e691eac60f0c668867d8579cd57
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1988,7 +1988,8 @@ AccessorDescriptor* DescriptorArray::GetCallbacks(int
descriptor_number) {
bool DescriptorArray::IsProperty(int descriptor_number) {
- return IsRealProperty(GetType(descriptor_number));
+ Entry entry(this, descriptor_number);
+ return IsPropertyDescriptor(&entry);
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
be75faba65c0d58183868a185d7c4e6e3ff523da..364874e03ddb81005a0791dcb2212d118ec4b6b2
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -2589,6 +2589,20 @@ class DescriptorArray: public FixedArray {
static const int kMaxNumberOfDescriptors = 1024 + 512;
private:
+ // An entry in a DescriptorArray, represented as a closure.
+ class Entry {
+ public:
+ inline explicit Entry(DescriptorArray* descs, int index) :
+ descs_(descs), index_(index) { }
+
+ inline PropertyType type() { return descs_->GetType(index_); }
+ inline Object* GetCallbackObject() { return descs_->GetValue(index_); }
+
+ private:
+ DescriptorArray* descs_;
+ int index_;
+ };
+
// Conversion from descriptor number to array indices.
static int ToKeyIndex(int descriptor_number) {
return descriptor_number+kFirstIndex;
@@ -7919,6 +7933,10 @@ class AccessorPair: public Struct {
}
}
+ bool ContainsAccessor() {
+ return IsJSAccessor(getter()) || IsJSAccessor(setter());
+ }
+
#ifdef OBJECT_PRINT
void AccessorPairPrint(FILE* out = stdout);
#endif
@@ -7931,6 +7949,15 @@ class AccessorPair: public Struct {
static const int kSize = kSetterOffset + kPointerSize;
private:
+ // Strangely enough, in addition to functions and harmony proxies, the
spec
+ // requires us to consider undefined as a kind of accessor, too:
+ // var obj = {};
+ // Object.defineProperty(obj, "foo", {get: undefined});
+ // assertTrue("foo" in obj);
+ bool IsJSAccessor(Object* obj) {
+ return obj->IsSpecFunction() || obj->IsUndefined();
+ }
+
DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorPair);
};
Index: src/property-details.h
diff --git a/src/property-details.h b/src/property-details.h
index
81f521a627ac77457675ef9fb1f4dd8e55dfaa37..c79aa969d344d1e795d79d9deb63329561c90fd3
100644
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -73,26 +73,6 @@ enum PropertyType {
};
-inline bool IsRealProperty(PropertyType type) {
- switch (type) {
- case NORMAL:
- case FIELD:
- case CONSTANT_FUNCTION:
- case CALLBACKS:
- case HANDLER:
- case INTERCEPTOR:
- return true;
- case MAP_TRANSITION:
- case ELEMENTS_TRANSITION:
- case CONSTANT_TRANSITION:
- case NULL_DESCRIPTOR:
- return false;
- }
- UNREACHABLE(); // keep the compiler happy
- return false;
-}
-
-
// PropertyDetails captures type and attributes for a property.
// They are used both in property dictionaries and instance descriptors.
class PropertyDetails BASE_EMBEDDED {
Index: src/property.h
diff --git a/src/property.h b/src/property.h
index
d5efb7f35193f9a5ee36dbe9339e968996aaf4d0..9235c3287dc74a302fb61276d3352c6f4587f2d9
100644
--- a/src/property.h
+++ b/src/property.h
@@ -164,6 +164,35 @@ class CallbacksDescriptor: public Descriptor {
};
+template <class T>
+bool IsPropertyDescriptor(T* desc) {
+ switch (desc->type()) {
+ case NORMAL:
+ case FIELD:
+ case CONSTANT_FUNCTION:
+ case HANDLER:
+ case INTERCEPTOR:
+ return true;
+ case CALLBACKS: {
+ Object* callback_object = desc->GetCallbackObject();
+ // Non-JavaScript (i.e. native) accessors are always a property,
otherwise
+ // either the getter or the setter must be an accessor. Put another
way:
+ // If we only see map transitions and holes in a pair, this is not a
+ // property.
+ return (!callback_object->IsAccessorPair() ||
+ AccessorPair::cast(callback_object)->ContainsAccessor());
+ }
+ case MAP_TRANSITION:
+ case ELEMENTS_TRANSITION:
+ case CONSTANT_TRANSITION:
+ case NULL_DESCRIPTOR:
+ return false;
+ }
+ UNREACHABLE(); // keep the compiler happy
+ return false;
+}
+
+
class LookupResult BASE_EMBEDDED {
public:
explicit LookupResult(Isolate* isolate)
@@ -261,10 +290,9 @@ class LookupResult BASE_EMBEDDED {
bool IsFound() { return lookup_type_ != NOT_FOUND; }
bool IsHandler() { return lookup_type_ == HANDLER_TYPE; }
- // Is the result is a property excluding transitions and the null
- // descriptor?
+ // Is the result is a property excluding transitions and the null
descriptor?
bool IsProperty() {
- return IsFound() && IsRealProperty(GetPropertyDetails().type());
+ return IsFound() && IsPropertyDescriptor(this);
}
bool IsCacheable() { return cacheable_; }
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
