Commit bebfe4db authored by barraclough@apple.com's avatar barraclough@apple.com

Merge 'Getter'/'Setter' attributes into 'Accessor'

https://bugs.webkit.org/show_bug.cgi?id=76141

Reviewed by Filip Pizlo.

These are currently ambiguous (and used inconsistently). It would logically appear
that either being bit set implies that the corresponding type of accessor is present
but (a) we don't correctly enforce this, and (b) this means the attributes would not
be able to distinguish between a data descriptor and an accessor descriptor with
neither a getter nor setter defined (which is a descriptor permissible under the spec).
This ambiguity would lead to unsafe property caching behavior (though this does not
represent an actual current bug, since we are currently unable to create descriptors
that have neither a getter nor setter, it just prevents us from doing so).

* runtime/Arguments.cpp:
(JSC::Arguments::createStrictModeCallerIfNecessary):
(JSC::Arguments::createStrictModeCalleeIfNecessary):
* runtime/JSArray.cpp:
(JSC::SparseArrayValueMap::put):
(JSC::JSArray::putDescriptor):
* runtime/JSBoundFunction.cpp:
(JSC::JSBoundFunction::finishCreation):
* runtime/JSFunction.cpp:
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::getOwnPropertyDescriptor):
* runtime/JSObject.cpp:
(JSC::JSObject::defineGetter):
(JSC::JSObject::initializeGetterSetterProperty):
(JSC::JSObject::defineSetter):
(JSC::putDescriptor):
(JSC::JSObject::defineOwnProperty):
* runtime/JSObject.h:
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorDefineProperty):
* runtime/PropertyDescriptor.cpp:
(JSC::PropertyDescriptor::setDescriptor):
(JSC::PropertyDescriptor::setAccessorDescriptor):
(JSC::PropertyDescriptor::setSetter):
(JSC::PropertyDescriptor::setGetter):
(JSC::PropertyDescriptor::attributesOverridingCurrent):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@104784 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 25d7148a
2012-01-11 Gavin Barraclough <barraclough@apple.com>
Merge 'Getter'/'Setter' attributes into 'Accessor'
https://bugs.webkit.org/show_bug.cgi?id=76141
Reviewed by Filip Pizlo.
These are currently ambiguous (and used inconsistently). It would logically appear
that either being bit set implies that the corresponding type of accessor is present
but (a) we don't correctly enforce this, and (b) this means the attributes would not
be able to distinguish between a data descriptor and an accessor descriptor with
neither a getter nor setter defined (which is a descriptor permissible under the spec).
This ambiguity would lead to unsafe property caching behavior (though this does not
represent an actual current bug, since we are currently unable to create descriptors
that have neither a getter nor setter, it just prevents us from doing so).
* runtime/Arguments.cpp:
(JSC::Arguments::createStrictModeCallerIfNecessary):
(JSC::Arguments::createStrictModeCalleeIfNecessary):
* runtime/JSArray.cpp:
(JSC::SparseArrayValueMap::put):
(JSC::JSArray::putDescriptor):
* runtime/JSBoundFunction.cpp:
(JSC::JSBoundFunction::finishCreation):
* runtime/JSFunction.cpp:
(JSC::JSFunction::getOwnPropertySlot):
(JSC::JSFunction::getOwnPropertyDescriptor):
* runtime/JSObject.cpp:
(JSC::JSObject::defineGetter):
(JSC::JSObject::initializeGetterSetterProperty):
(JSC::JSObject::defineSetter):
(JSC::putDescriptor):
(JSC::JSObject::defineOwnProperty):
* runtime/JSObject.h:
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorDefineProperty):
* runtime/PropertyDescriptor.cpp:
(JSC::PropertyDescriptor::setDescriptor):
(JSC::PropertyDescriptor::setAccessorDescriptor):
(JSC::PropertyDescriptor::setSetter):
(JSC::PropertyDescriptor::setGetter):
(JSC::PropertyDescriptor::attributesOverridingCurrent):
2012-01-11 Gavin Barraclough <barraclough@apple.com>
Object.defineProperty([], 'length', {}) should not make length read-only
......@@ -109,7 +109,7 @@ void Arguments::createStrictModeCallerIfNecessary(ExecState* exec)
d->overrodeCaller = true;
PropertyDescriptor descriptor;
descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Getter | Setter);
descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Accessor);
methodTable()->defineOwnProperty(this, exec, exec->propertyNames().caller, descriptor, false);
}
......@@ -120,7 +120,7 @@ void Arguments::createStrictModeCalleeIfNecessary(ExecState* exec)
d->overrodeCallee = true;
PropertyDescriptor descriptor;
descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Getter | Setter);
descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Accessor);
methodTable()->defineOwnProperty(this, exec, exec->propertyNames().callee, descriptor, false);
}
......
......@@ -229,7 +229,7 @@ inline void SparseArrayValueMap::put(ExecState* exec, JSArray* array, unsigned i
{
SparseArrayEntry& entry = add(array, i).first->second;
if (!(entry.attributes & (Getter | Setter))) {
if (!(entry.attributes & Accessor)) {
if (entry.attributes & ReadOnly) {
// FIXME: should throw if being called from strict mode.
// throwTypeError(exec, StrictModeReadonlyPropertyWriteError);
......@@ -348,7 +348,7 @@ void JSArray::putDescriptor(ExecState* exec, SparseArrayEntry* entryInMap, Prope
entryInMap->set(exec->globalData(), this, descriptor.value());
else if (oldDescriptor.isAccessorDescriptor())
entryInMap->set(exec->globalData(), this, jsUndefined());
entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~(Getter | Setter);
entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~Accessor;
return;
}
......
......@@ -111,8 +111,8 @@ void JSBoundFunction::finishCreation(ExecState* exec, NativeExecutable* executab
Base::finishCreation(exec, executable, length, name);
ASSERT(inherits(&s_info));
initializeGetterSetterProperty(exec, exec->propertyNames().arguments, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
initializeGetterSetterProperty(exec, exec->propertyNames().caller, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
initializeGetterSetterProperty(exec, exec->propertyNames().arguments, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
initializeGetterSetterProperty(exec, exec->propertyNames().caller, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
}
void JSBoundFunction::visitChildren(JSCell* cell, SlotVisitor& visitor)
......
......@@ -216,7 +216,7 @@ bool JSFunction::getOwnPropertySlot(JSCell* cell, ExecState* exec, const Identif
if (thisObject->jsExecutable()->isStrictMode()) {
bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
if (!result) {
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
ASSERT(result);
}
......@@ -235,7 +235,7 @@ bool JSFunction::getOwnPropertySlot(JSCell* cell, ExecState* exec, const Identif
if (thisObject->jsExecutable()->isStrictMode()) {
bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
if (!result) {
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
ASSERT(result);
}
......@@ -264,7 +264,7 @@ bool JSFunction::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, con
if (thisObject->jsExecutable()->isStrictMode()) {
bool result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
if (!result) {
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
ASSERT(result);
}
......@@ -283,7 +283,7 @@ bool JSFunction::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, con
if (thisObject->jsExecutable()->isStrictMode()) {
bool result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
if (!result) {
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
ASSERT(result);
}
......
......@@ -361,7 +361,7 @@ void JSObject::defineGetter(JSObject* thisObject, ExecState* exec, const Identif
JSGlobalData& globalData = exec->globalData();
PutPropertySlot slot;
GetterSetter* getterSetter = GetterSetter::create(exec);
thisObject->putDirectInternal(globalData, propertyName, getterSetter, attributes | Getter, true, slot, 0);
thisObject->putDirectInternal(globalData, propertyName, getterSetter, attributes | Accessor, true, slot, 0);
// putDirect will change our Structure if we add a new property. For
// getters and setters, though, we also need to change our Structure
......@@ -379,12 +379,11 @@ void JSObject::initializeGetterSetterProperty(ExecState* exec, const Identifier&
{
// Set an inital property on an object; the property must not already exist & the attribute flags must be set correctly.
ASSERT(structure()->get(exec->globalData(), propertyName) == WTF::notFound);
ASSERT(static_cast<bool>(getterSetter->getter()) == static_cast<bool>(attributes & Getter));
ASSERT(static_cast<bool>(getterSetter->setter()) == static_cast<bool>(attributes & Setter));
ASSERT(static_cast<bool>(attributes & Accessor));
JSGlobalData& globalData = exec->globalData();
PutPropertySlot slot;
putDirectInternal(globalData, propertyName, getterSetter, attributes | Getter, true, slot, 0);
putDirectInternal(globalData, propertyName, getterSetter, attributes, true, slot, 0);
// putDirect will change our Structure if we add a new property. For
// getters and setters, though, we also need to change our Structure
......@@ -413,7 +412,7 @@ void JSObject::defineSetter(JSObject* thisObject, ExecState* exec, const Identif
PutPropertySlot slot;
GetterSetter* getterSetter = GetterSetter::create(exec);
thisObject->putDirectInternal(exec->globalData(), propertyName, getterSetter, attributes | Setter, true, slot, 0);
thisObject->putDirectInternal(exec->globalData(), propertyName, getterSetter, attributes | Accessor, true, slot, 0);
// putDirect will change our Structure if we add a new property. For
// getters and setters, though, we also need to change our Structure
......@@ -693,11 +692,11 @@ static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& p
if (descriptor.isGenericDescriptor() && oldDescriptor.isAccessorDescriptor()) {
GetterSetter* accessor = GetterSetter::create(exec);
if (oldDescriptor.getter()) {
attributes |= Getter;
attributes |= Accessor;
accessor->setGetter(exec->globalData(), asObject(oldDescriptor.getter()));
}
if (oldDescriptor.setter()) {
attributes |= Setter;
attributes |= Accessor;
accessor->setSetter(exec->globalData(), asObject(oldDescriptor.setter()));
}
target->methodTable()->putWithAttributes(target, exec, propertyName, accessor, attributes);
......@@ -708,7 +707,7 @@ static bool putDescriptor(ExecState* exec, JSObject* target, const Identifier& p
newValue = descriptor.value();
else if (oldDescriptor.value())
newValue = oldDescriptor.value();
target->methodTable()->putWithAttributes(target, exec, propertyName, newValue, attributes & ~(Getter | Setter));
target->methodTable()->putWithAttributes(target, exec, propertyName, newValue, attributes & ~Accessor);
return true;
}
attributes &= ~ReadOnly;
......@@ -832,10 +831,8 @@ bool JSObject::defineOwnProperty(JSObject* object, ExecState* exec, const Identi
}
object->methodTable()->deleteProperty(object, exec, propertyName);
unsigned attrs = current.attributesWithOverride(descriptor);
if (descriptor.setter())
attrs |= Setter;
if (descriptor.getter())
attrs |= Getter;
if (descriptor.setter() || descriptor.getter())
attrs |= Accessor;
object->putDirect(exec->globalData(), propertyName, getterSetter, attrs);
return true;
}
......
......@@ -66,8 +66,7 @@ namespace JSC {
DontEnum = 1 << 2, // property doesn't appear in (for .. in ..)
DontDelete = 1 << 3, // property can't be deleted
Function = 1 << 4, // property is a function - only used by static hashtables
Getter = 1 << 5, // property is a getter
Setter = 1 << 6 // property is a setter
Accessor = 1 << 5, // property is a getter/setter
};
class JSObject : public JSCell {
......
......@@ -299,7 +299,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorDefineProperty(ExecState* exec)
PropertyDescriptor descriptor;
if (!toPropertyDescriptor(exec, exec->argument(2), descriptor))
return JSValue::encode(jsNull());
ASSERT((descriptor.attributes() & (Getter | Setter)) || (!descriptor.isAccessorDescriptor()));
ASSERT((descriptor.attributes() & Accessor) || (!descriptor.isAccessorDescriptor()));
ASSERT(!exec->hadException());
O->methodTable()->defineOwnProperty(O, exec, Identifier(exec, propertyName), descriptor, true);
return JSValue::encode(O);
......
......@@ -87,19 +87,16 @@ JSValue PropertyDescriptor::setter() const
void PropertyDescriptor::setDescriptor(JSValue value, unsigned attributes)
{
ASSERT(value);
ASSERT(value.isGetterSetter() == !!(attributes & Accessor));
m_attributes = attributes;
if (value.isGetterSetter()) {
GetterSetter* accessor = asGetterSetter(value);
m_getter = accessor->getter();
if (m_getter)
m_attributes |= Getter;
m_setter = accessor->setter();
if (m_setter)
m_attributes |= Setter;
ASSERT(m_getter || m_setter);
m_seenAttributes = EnumerablePresent | ConfigurablePresent;
m_attributes &= ~ReadOnly;
} else {
......@@ -110,10 +107,8 @@ void PropertyDescriptor::setDescriptor(JSValue value, unsigned attributes)
void PropertyDescriptor::setAccessorDescriptor(GetterSetter* accessor, unsigned attributes)
{
ASSERT(attributes & (Getter | Setter));
ASSERT(attributes & Accessor);
ASSERT(accessor->getter() || accessor->setter());
ASSERT(!accessor->getter() == !(attributes & Getter));
ASSERT(!accessor->setter() == !(attributes & Setter));
m_attributes = attributes;
m_getter = accessor->getter();
m_setter = accessor->setter();
......@@ -151,14 +146,14 @@ void PropertyDescriptor::setConfigurable(bool configurable)
void PropertyDescriptor::setSetter(JSValue setter)
{
m_setter = setter;
m_attributes |= Setter;
m_attributes |= Accessor;
m_attributes &= ~ReadOnly;
}
void PropertyDescriptor::setGetter(JSValue getter)
{
m_getter = getter;
m_attributes |= Getter;
m_attributes |= Accessor;
m_attributes &= ~ReadOnly;
}
......@@ -225,7 +220,7 @@ unsigned PropertyDescriptor::attributesOverridingCurrent(const PropertyDescripto
if (configurablePresent())
overrideMask |= DontDelete;
if (isAccessorDescriptor())
overrideMask |= (Getter | Setter);
overrideMask |= Accessor;
return (m_attributes & overrideMask) | (current.m_attributes & ~overrideMask);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment