Changeset 104784 in webkit


Ignore:
Timestamp:
Jan 11, 2012 7:53:22 PM (12 years ago)
Author:
barraclough@apple.com
Message:

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):

Location:
trunk/Source/JavaScriptCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r104777 r104784  
     12012-01-11  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Merge 'Getter'/'Setter' attributes into 'Accessor'
     4        https://bugs.webkit.org/show_bug.cgi?id=76141
     5
     6        Reviewed by Filip Pizlo.
     7
     8        These are currently ambiguous (and used inconsistently). It would logically appear
     9        that either being bit set implies that the corresponding type of accessor is present
     10        but (a) we don't correctly enforce this, and (b) this means the attributes would not
     11        be able to distinguish between a data descriptor and an accessor descriptor with
     12        neither a getter nor setter defined (which is a descriptor permissible under the spec).
     13        This ambiguity would lead to unsafe property caching behavior (though this does not
     14        represent an actual current bug, since we are currently unable to create descriptors
     15        that have neither a getter nor setter, it just prevents us from doing so).
     16
     17        * runtime/Arguments.cpp:
     18        (JSC::Arguments::createStrictModeCallerIfNecessary):
     19        (JSC::Arguments::createStrictModeCalleeIfNecessary):
     20        * runtime/JSArray.cpp:
     21        (JSC::SparseArrayValueMap::put):
     22        (JSC::JSArray::putDescriptor):
     23        * runtime/JSBoundFunction.cpp:
     24        (JSC::JSBoundFunction::finishCreation):
     25        * runtime/JSFunction.cpp:
     26        (JSC::JSFunction::getOwnPropertySlot):
     27        (JSC::JSFunction::getOwnPropertyDescriptor):
     28        * runtime/JSObject.cpp:
     29        (JSC::JSObject::defineGetter):
     30        (JSC::JSObject::initializeGetterSetterProperty):
     31        (JSC::JSObject::defineSetter):
     32        (JSC::putDescriptor):
     33        (JSC::JSObject::defineOwnProperty):
     34        * runtime/JSObject.h:
     35        * runtime/ObjectConstructor.cpp:
     36        (JSC::objectConstructorDefineProperty):
     37        * runtime/PropertyDescriptor.cpp:
     38        (JSC::PropertyDescriptor::setDescriptor):
     39        (JSC::PropertyDescriptor::setAccessorDescriptor):
     40        (JSC::PropertyDescriptor::setSetter):
     41        (JSC::PropertyDescriptor::setGetter):
     42        (JSC::PropertyDescriptor::attributesOverridingCurrent):
     43
    1442012-01-11  Gavin Barraclough  <barraclough@apple.com>
    245

  • trunk/Source/JavaScriptCore/runtime/Arguments.cpp

    r104119 r104784  
    110110    d->overrodeCaller = true;
    111111    PropertyDescriptor descriptor;
    112     descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Getter | Setter);
     112    descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Accessor);
    113113    methodTable()->defineOwnProperty(this, exec, exec->propertyNames().caller, descriptor, false);
    114114}
    … …  
    121121    d->overrodeCallee = true;
    122122    PropertyDescriptor descriptor;
    123     descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Getter | Setter);
     123    descriptor.setAccessorDescriptor(globalObject()->throwTypeErrorGetterSetter(exec), DontEnum | DontDelete | Accessor);
    124124    methodTable()->defineOwnProperty(this, exec, exec->propertyNames().callee, descriptor, false);
    125125}

  • trunk/Source/JavaScriptCore/runtime/JSArray.cpp

    r104777 r104784  
    230230    SparseArrayEntry& entry = add(array, i).first->second;
    231231
    232     if (!(entry.attributes & (Getter | Setter))) {
     232    if (!(entry.attributes & Accessor)) {
    233233        if (entry.attributes & ReadOnly) {
    234234            // FIXME: should throw if being called from strict mode.
    … …  
    349349        else if (oldDescriptor.isAccessorDescriptor())
    350350            entryInMap->set(exec->globalData(), this, jsUndefined());
    351         entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~(Getter | Setter);
     351        entryInMap->attributes = descriptor.attributesOverridingCurrent(oldDescriptor) & ~Accessor;
    352352        return;
    353353    }

  • trunk/Source/JavaScriptCore/runtime/JSBoundFunction.cpp

    r103243 r104784  
    112112    ASSERT(inherits(&s_info));
    113113
    114     initializeGetterSetterProperty(exec, exec->propertyNames().arguments, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
    115     initializeGetterSetterProperty(exec, exec->propertyNames().caller, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
     114    initializeGetterSetterProperty(exec, exec->propertyNames().arguments, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
     115    initializeGetterSetterProperty(exec, exec->propertyNames().caller, globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
    116116}
    117117

  • trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

    r103958 r104784  
    217217            bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
    218218            if (!result) {
    219                 thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
     219                thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
    220220                result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
    221221                ASSERT(result);
    … …  
    236236            bool result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
    237237            if (!result) {
    238                 thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
     238                thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
    239239                result = Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
    240240                ASSERT(result);
    … …  
    265265            bool result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
    266266            if (!result) {
    267                 thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
     267                thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
    268268                result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
    269269                ASSERT(result);
    … …  
    284284            bool result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
    285285            if (!result) {
    286                 thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Getter | Setter);
     286                thisObject->initializeGetterSetterProperty(exec, propertyName, thisObject->globalObject()->throwTypeErrorGetterSetter(exec), DontDelete | DontEnum | Accessor);
    287287                result = Base::getOwnPropertyDescriptor(thisObject, exec, propertyName, descriptor);
    288288                ASSERT(result);

  • trunk/Source/JavaScriptCore/runtime/JSObject.cpp

    r104602 r104784  
    362362    PutPropertySlot slot;
    363363    GetterSetter* getterSetter = GetterSetter::create(exec);
    364     thisObject->putDirectInternal(globalData, propertyName, getterSetter, attributes | Getter, true, slot, 0);
     364    thisObject->putDirectInternal(globalData, propertyName, getterSetter, attributes | Accessor, true, slot, 0);
    365365
    366366    // putDirect will change our Structure if we add a new property. For
    … …  
    380380    // Set an inital property on an object; the property must not already exist & the attribute flags must be set correctly.
    381381    ASSERT(structure()->get(exec->globalData(), propertyName) == WTF::notFound);
    382     ASSERT(static_cast<bool>(getterSetter->getter()) == static_cast<bool>(attributes & Getter));
    383     ASSERT(static_cast<bool>(getterSetter->setter()) == static_cast<bool>(attributes & Setter));
     382    ASSERT(static_cast<bool>(attributes & Accessor));
    384383
    385384    JSGlobalData& globalData = exec->globalData();
    386385    PutPropertySlot slot;
    387     putDirectInternal(globalData, propertyName, getterSetter, attributes | Getter, true, slot, 0);
     386    putDirectInternal(globalData, propertyName, getterSetter, attributes, true, slot, 0);
    388387
    389388    // putDirect will change our Structure if we add a new property. For
    … …  
    414413    PutPropertySlot slot;
    415414    GetterSetter* getterSetter = GetterSetter::create(exec);
    416     thisObject->putDirectInternal(exec->globalData(), propertyName, getterSetter, attributes | Setter, true, slot, 0);
     415    thisObject->putDirectInternal(exec->globalData(), propertyName, getterSetter, attributes | Accessor, true, slot, 0);
    417416
    418417    // putDirect will change our Structure if we add a new property. For
    … …  
    694693            GetterSetter* accessor = GetterSetter::create(exec);
    695694            if (oldDescriptor.getter()) {
    696                 attributes |= Getter;
     695                attributes |= Accessor;
    697696                accessor->setGetter(exec->globalData(), asObject(oldDescriptor.getter()));
    698697            }
    699698            if (oldDescriptor.setter()) {
    700                 attributes |= Setter;
     699                attributes |= Accessor;
    701700                accessor->setSetter(exec->globalData(), asObject(oldDescriptor.setter()));
    702701            }
    … …  
    709708        else if (oldDescriptor.value())
    710709            newValue = oldDescriptor.value();
    711         target->methodTable()->putWithAttributes(target, exec, propertyName, newValue, attributes & ~(Getter | Setter));
     710        target->methodTable()->putWithAttributes(target, exec, propertyName, newValue, attributes & ~Accessor);
    712711        return true;
    713712    }
    … …  
    833832    object->methodTable()->deleteProperty(object, exec, propertyName);
    834833    unsigned attrs = current.attributesWithOverride(descriptor);
    835     if (descriptor.setter())
    836         attrs |= Setter;
    837     if (descriptor.getter())
    838         attrs |= Getter;
     834    if (descriptor.setter() || descriptor.getter())
     835        attrs |= Accessor;
    839836    object->putDirect(exec->globalData(), propertyName, getterSetter, attrs);
    840837    return true;

  • trunk/Source/JavaScriptCore/runtime/JSObject.h

    r103698 r104784  
    6767        DontDelete   = 1 << 3,  // property can't be deleted
    6868        Function     = 1 << 4,  // property is a function - only used by static hashtables
    69         Getter       = 1 << 5,  // property is a getter
    70         Setter       = 1 << 6   // property is a setter
     69        Accessor     = 1 << 5,  // property is a getter/setter
    7170    };
    7271

  • trunk/Source/JavaScriptCore/runtime/ObjectConstructor.cpp

    r103083 r104784  
    300300    if (!toPropertyDescriptor(exec, exec->argument(2), descriptor))
    301301        return JSValue::encode(jsNull());
    302     ASSERT((descriptor.attributes() & (Getter | Setter)) || (!descriptor.isAccessorDescriptor()));
     302    ASSERT((descriptor.attributes() & Accessor) || (!descriptor.isAccessorDescriptor()));
    303303    ASSERT(!exec->hadException());
    304304    O->methodTable()->defineOwnProperty(O, exec, Identifier(exec, propertyName), descriptor, true);

  • trunk/Source/JavaScriptCore/runtime/PropertyDescriptor.cpp

    r104602 r104784  
    8888{
    8989    ASSERT(value);
     90    ASSERT(value.isGetterSetter() == !!(attributes & Accessor));
     91
    9092    m_attributes = attributes;
    9193    if (value.isGetterSetter()) {
    … …  
    9395
    9496        m_getter = accessor->getter();
    95         if (m_getter)
    96             m_attributes |= Getter;
    97 
    9897        m_setter = accessor->setter();
    99         if (m_setter)
    100             m_attributes |= Setter;
    101 
    10298        ASSERT(m_getter || m_setter);
     99
    103100        m_seenAttributes = EnumerablePresent | ConfigurablePresent;
    104101        m_attributes &= ~ReadOnly;
    … …  
    111108void PropertyDescriptor::setAccessorDescriptor(GetterSetter* accessor, unsigned attributes)
    112109{
    113     ASSERT(attributes & (Getter | Setter));
     110    ASSERT(attributes & Accessor);
    114111    ASSERT(accessor->getter() || accessor->setter());
    115     ASSERT(!accessor->getter() == !(attributes & Getter));
    116     ASSERT(!accessor->setter() == !(attributes & Setter));
    117112    m_attributes = attributes;
    118113    m_getter = accessor->getter();
    … …  
    152147{
    153148    m_setter = setter;
    154     m_attributes |= Setter;
     149    m_attributes |= Accessor;
    155150    m_attributes &= ~ReadOnly;
    156151}
    … …  
    159154{
    160155    m_getter = getter;
    161     m_attributes |= Getter;
     156    m_attributes |= Accessor;
    162157    m_attributes &= ~ReadOnly;
    163158}
    … …  
    226221        overrideMask |= DontDelete;
    227222    if (isAccessorDescriptor())
    228         overrideMask |= (Getter | Setter);
     223        overrideMask |= Accessor;
    229224    return (m_attributes & overrideMask) | (current.m_attributes & ~overrideMask);
    230225}
Note: See TracChangeset for help on using the changeset viewer.