Removed getDirectLocation and offsetForLocation and all their uses

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

Reviewed by Filip Pizlo.

getDirectLocation() and its associated offsetForLocation() relied on
detailed knowledge of the rules of PropertyOffset, JSObject, and
Structure, which is a hard thing to reverse-engineer reliably. Luckily,
it wasn't needed, and all clients either wanted a true value or a
PropertyOffset. So, I refactored accordingly.

* dfg/DFGOperations.cpp: Renamed putDirectOffset to putDirect, to clarify
that we are not putting an offset.

* runtime/JSActivation.cpp:
(JSC::JSActivation::getOwnPropertySlot): Get a value instead of a value
pointer, since we never wanted a pointer to begin with.

* runtime/JSFunction.cpp:
(JSC::JSFunction::getOwnPropertySlot): Use a PropertyOffset instead of a pointer,
so we don't have to reverse-engineer the offset from the pointer.

* runtime/JSObject.cpp:
(JSC::JSObject::put):
(JSC::JSObject::resetInheritorID):
(JSC::JSObject::inheritorID):
(JSC::JSObject::removeDirect):
(JSC::JSObject::fillGetterPropertySlot):
(JSC::JSObject::getOwnPropertyDescriptor): Renamed getDirectOffset and
putDirectOffset, as explaind above. We want to use the name "getDirectOffset"
for when the thing you're getting is the offset.

* runtime/JSObject.h:
(JSC::JSObject::getDirect):
(JSC::JSObject::getDirectOffset): Changed getDirectLocation to getDirectOffset,
since clients really wants PropertyOffsets and not locations.

(JSObject::offsetForLocation): Removed this function because it was hard
to get right.

(JSC::JSObject::putDirect):
(JSC::JSObject::putDirectUndefined):
(JSC::JSObject::inlineGetOwnPropertySlot):
(JSC::JSObject::putDirectInternal):
(JSC::JSObject::putDirectWithoutTransition):
* runtime/JSScope.cpp:
(JSC::executeResolveOperations):
(JSC::JSScope::resolvePut):
* runtime/JSValue.cpp:
(JSC::JSValue::putToPrimitive): Updated for renames.

* runtime/Lookup.cpp:
(JSC::setUpStaticFunctionSlot): Use a PropertyOffset instead of a pointer,
so we don't have to reverse-engineer the offset from the pointer.

* runtime/Structure.cpp:
(JSC::Structure::flattenDictionaryStructure): Updated for renames.


Canonical link: https://commits.webkit.org/124906@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139491 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,63 @@
+2013-01-11  Geoffrey Garen  <ggaren@apple.com>
+
+        Removed getDirectLocation and offsetForLocation and all their uses
+        https://bugs.webkit.org/show_bug.cgi?id=106692
+
+        Reviewed by Filip Pizlo.
+
+        getDirectLocation() and its associated offsetForLocation() relied on
+        detailed knowledge of the rules of PropertyOffset, JSObject, and
+        Structure, which is a hard thing to reverse-engineer reliably. Luckily,
+        it wasn't needed, and all clients either wanted a true value or a
+        PropertyOffset. So, I refactored accordingly.
+
+        * dfg/DFGOperations.cpp: Renamed putDirectOffset to putDirect, to clarify
+        that we are not putting an offset.
+
+        * runtime/JSActivation.cpp:
+        (JSC::JSActivation::getOwnPropertySlot): Get a value instead of a value
+        pointer, since we never wanted a pointer to begin with.
+
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::getOwnPropertySlot): Use a PropertyOffset instead of a pointer,
+        so we don't have to reverse-engineer the offset from the pointer.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::put):
+        (JSC::JSObject::resetInheritorID):
+        (JSC::JSObject::inheritorID):
+        (JSC::JSObject::removeDirect):
+        (JSC::JSObject::fillGetterPropertySlot):
+        (JSC::JSObject::getOwnPropertyDescriptor): Renamed getDirectOffset and
+        putDirectOffset, as explaind above. We want to use the name "getDirectOffset"
+        for when the thing you're getting is the offset.
+
+        * runtime/JSObject.h:
+        (JSC::JSObject::getDirect):
+        (JSC::JSObject::getDirectOffset): Changed getDirectLocation to getDirectOffset,
+        since clients really wants PropertyOffsets and not locations.
+
+        (JSObject::offsetForLocation): Removed this function because it was hard
+        to get right.
+
+        (JSC::JSObject::putDirect):
+        (JSC::JSObject::putDirectUndefined):
+        (JSC::JSObject::inlineGetOwnPropertySlot):
+        (JSC::JSObject::putDirectInternal):
+        (JSC::JSObject::putDirectWithoutTransition):
+        * runtime/JSScope.cpp:
+        (JSC::executeResolveOperations):
+        (JSC::JSScope::resolvePut):
+        * runtime/JSValue.cpp:
+        (JSC::JSValue::putToPrimitive): Updated for renames.
+
+        * runtime/Lookup.cpp:
+        (JSC::setUpStaticFunctionSlot): Use a PropertyOffset instead of a pointer,
+        so we don't have to reverse-engineer the offset from the pointer.
+
+        * runtime/Structure.cpp:
+        (JSC::Structure::flattenDictionaryStructure): Updated for renames.
+
 2013-01-11  Geoffrey Garen  <ggaren@apple.com>
 
         Removed an unused version of getDirectLocation

Source/JavaScriptCore/dfg/DFGOperations.cpp

@@ -1441,7 +1441,7 @@ void DFG_OPERATION operationReallocateStorageAndFinishPut(ExecState* exec, JSObj
     ASSERT(structure->outOfLineCapacity() > base->structure()->outOfLineCapacity());
     ASSERT(!globalData.heap.storageAllocator().fastPathShouldSucceed(structure->outOfLineCapacity() * sizeof(JSValue)));
     base->setStructureAndReallocateStorageIfNecessary(globalData, structure);
-    base->putDirectOffset(globalData, offset, JSValue::decode(value));
+    base->putDirect(globalData, offset, JSValue::decode(value));
 }
 
 char* DFG_OPERATION operationAllocatePropertyStorageWithInitialCapacity(ExecState* exec)

Source/JavaScriptCore/runtime/JSActivation.cpp

@@ -156,8 +156,8 @@ bool JSActivation::getOwnPropertySlot(JSCell* cell, ExecState* exec, PropertyNam
     if (thisObject->symbolTableGet(propertyName, slot))
         return true;
 
-    if (WriteBarrierBase<Unknown>* location = thisObject->getDirectLocation(exec->globalData(), propertyName)) {
-        slot.setValue(location->get());
+    if (JSValue value = thisObject->getDirect(exec->globalData(), propertyName)) {
+        slot.setValue(value);
         return true;
     }
 

Source/JavaScriptCore/runtime/JSFunction.cpp

@@ -218,16 +218,16 @@ bool JSFunction::getOwnPropertySlot(JSCell* cell, ExecState* exec, PropertyName
         return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
 
     if (propertyName == exec->propertyNames().prototype) {
-        WriteBarrierBase<Unknown>* location = thisObject->getDirectLocation(exec->globalData(), propertyName);
-
-        if (!location) {
+        PropertyOffset offset = thisObject->getDirectOffset(exec->globalData(), propertyName);
+        if (!isValidOffset(offset)) {
             JSObject* prototype = constructEmptyObject(exec, thisObject->globalObject()->emptyObjectStructure());
             prototype->putDirect(exec->globalData(), exec->propertyNames().constructor, thisObject, DontEnum);
             thisObject->putDirect(exec->globalData(), exec->propertyNames().prototype, prototype, DontDelete | DontEnum);
-            location = thisObject->getDirectLocation(exec->globalData(), exec->propertyNames().prototype);
+            offset = thisObject->getDirectOffset(exec->globalData(), exec->propertyNames().prototype);
+            ASSERT(isValidOffset(offset));
         }
 
-        slot.setValue(thisObject, location->get(), thisObject->offsetForLocation(location));
+        slot.setValue(thisObject, thisObject->getDirect(offset), offset);
     }
 
     if (propertyName == exec->propertyNames().arguments) {

Source/JavaScriptCore/runtime/JSObject.cpp

@@ -379,7 +379,7 @@ void JSObject::put(JSCell* cell, ExecState* exec, PropertyName propertyName, JSV
                 return;
             }
 
-            JSValue gs = obj->getDirectOffset(offset);
+            JSValue gs = obj->getDirect(offset);
             if (gs.isGetterSetter()) {
                 ASSERT(attributes & Accessor);
                 ASSERT(thisObject->structure()->prototypeChainMayInterceptStoreTo(exec->globalData(), propertyName) || obj == thisObject);
@@ -1193,13 +1193,12 @@ void JSObject::resetInheritorID(JSGlobalData& globalData)
     if (!isValidOffset(offset))
         return;
 
-    putDirectOffset(globalData, offset, jsUndefined());
+    putDirect(globalData, offset, jsUndefined());
 }
 
 Structure* JSObject::inheritorID(JSGlobalData& globalData)
 {
-    if (WriteBarrierBase<Unknown>* location = getDirectLocation(globalData, globalData.m_inheritorIDKey)) {
-        JSValue value = location->get();
+    if (JSValue value = getDirect(globalData, globalData.m_inheritorIDKey)) {
         if (value.isCell()) {
             Structure* inheritorID = jsCast<Structure*>(value);
             ASSERT(inheritorID->isEmpty());
@@ -1642,20 +1641,20 @@ bool JSObject::removeDirect(JSGlobalData& globalData, PropertyName propertyName)
         offset = structure()->removePropertyWithoutTransition(globalData, propertyName);
         if (offset == invalidOffset)
             return false;
-        putUndefinedAtDirectOffset(offset);
+        putDirectUndefined(offset);
         return true;
     }
 
     setStructure(globalData, Structure::removePropertyTransition(globalData, structure(), propertyName, offset));
     if (offset == invalidOffset)
         return false;
-    putUndefinedAtDirectOffset(offset);
+    putDirectUndefined(offset);
     return true;
 }
 
 NEVER_INLINE void JSObject::fillGetterPropertySlot(PropertySlot& slot, PropertyOffset offset)
 {
-    if (JSObject* getterFunction = asGetterSetter(getDirectOffset(offset))->getter()) {
+    if (JSObject* getterFunction = asGetterSetter(getDirect(offset))->getter()) {
         if (!structure()->isDictionary())
             slot.setCacheableGetterSlot(this, getterFunction, offset);
         else
@@ -2428,7 +2427,7 @@ bool JSObject::getOwnPropertyDescriptor(JSObject* object, ExecState* exec, Prope
     JSCell* cell = 0;
     PropertyOffset offset = object->structure()->get(exec->globalData(), propertyName, attributes, cell);
     if (isValidOffset(offset)) {
-        descriptor.setDescriptor(object->getDirectOffset(offset), attributes);
+        descriptor.setDescriptor(object->getDirect(offset), attributes);
         return true;
     }
 

Source/JavaScriptCore/runtime/JSObject.h

@@ -505,14 +505,14 @@ class JSObject : public JSCell {
     {
         PropertyOffset offset = structure()->get(globalData, propertyName);
         checkOffset(offset, structure()->inlineCapacity());
-        return offset != invalidOffset ? getDirectOffset(offset) : JSValue();
+        return offset != invalidOffset ? getDirect(offset) : JSValue();
     }
 
-    WriteBarrierBase<Unknown>* getDirectLocation(JSGlobalData& globalData, PropertyName propertyName)
+    PropertyOffset getDirectOffset(JSGlobalData& globalData, PropertyName propertyName)
     {
         PropertyOffset offset = structure()->get(globalData, propertyName);
         checkOffset(offset, structure()->inlineCapacity());
-        return isValidOffset(offset) ? locationForOffset(offset) : 0;
+        return offset;
     }
 
     bool hasInlineStorage() const { return structure()->hasInlineStorage(); }
@@ -555,18 +555,6 @@ class JSObject : public JSCell {
         return &outOfLineStorage()[offsetInOutOfLineStorage(offset)];
     }
 
-    PropertyOffset offsetForLocation(WriteBarrierBase<Unknown>* location) const
-    {
-        PropertyOffset result;
-        size_t offsetInInlineStorage = location - inlineStorageUnsafe();
-        if (offsetInInlineStorage < static_cast<size_t>(firstOutOfLineOffset))
-            result = offsetInInlineStorage;
-        else
-            result = outOfLineStorage() - location + (firstOutOfLineOffset - 1);
-        validateOffset(result, structure()->inlineCapacity());
-        return result;
-    }
-
     void transitionTo(JSGlobalData&, Structure*);
 
     bool removeDirect(JSGlobalData&, PropertyName); // Return true if anything is removed.
@@ -581,9 +569,9 @@ class JSObject : public JSCell {
     bool putOwnDataProperty(JSGlobalData&, PropertyName, JSValue, PutPropertySlot&);
 
     // Fast access to known property offsets.
-    JSValue getDirectOffset(PropertyOffset offset) const { return locationForOffset(offset)->get(); }
-    void putDirectOffset(JSGlobalData& globalData, PropertyOffset offset, JSValue value) { locationForOffset(offset)->set(globalData, this, value); }
-    void putUndefinedAtDirectOffset(PropertyOffset offset) { locationForOffset(offset)->setUndefined(); }
+    JSValue getDirect(PropertyOffset offset) const { return locationForOffset(offset)->get(); }
+    void putDirect(JSGlobalData& globalData, PropertyOffset offset, JSValue value) { locationForOffset(offset)->set(globalData, this, value); }
+    void putDirectUndefined(PropertyOffset offset) { locationForOffset(offset)->setUndefined(); }
 
     JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, PropertyDescriptor&, bool shouldThrow);
 
@@ -1189,7 +1177,7 @@ ALWAYS_INLINE bool JSObject::inlineGetOwnPropertySlot(ExecState* exec, PropertyN
 {
     PropertyOffset offset = structure()->get(exec->globalData(), propertyName);
     if (LIKELY(isValidOffset(offset))) {
-        JSValue value = getDirectOffset(offset);
+        JSValue value = getDirect(offset);
         if (structure()->hasGetterSetterProperties() && value.isGetterSetter())
             fillGetterPropertySlot(slot, offset);
         else
@@ -1297,7 +1285,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
             if ((mode == PutModePut) && currentAttributes & ReadOnly)
                 return false;
 
-            putDirectOffset(globalData, offset, value);
+            putDirect(globalData, offset, value);
             // At this point, the objects structure only has a specific value set if previously there
             // had been one set, and if the new value being specified is the same (otherwise we would
             // have despecified, above).  So, if currentSpecificFunction is not set, or if the new
@@ -1320,7 +1308,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
 
         validateOffset(offset);
         ASSERT(structure()->isValidOffset(offset));
-        putDirectOffset(globalData, offset, value);
+        putDirect(globalData, offset, value);
         // See comment on setNewProperty call below.
         if (!specificFunction)
             slot.setNewProperty(this, offset);
@@ -1339,7 +1327,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
         validateOffset(offset);
         ASSERT(structure->isValidOffset(offset));
         setButterfly(globalData, newButterfly, structure);
-        putDirectOffset(globalData, offset, value);
+        putDirect(globalData, offset, value);
         // This is a new property; transitions with specific values are not currently cachable,
         // so leave the slot in an uncachable state.
         if (!specificFunction)
@@ -1366,7 +1354,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
         if (currentSpecificFunction) {
             // case (1) Do the put, then return leaving the slot uncachable.
             if (specificFunction == currentSpecificFunction) {
-                putDirectOffset(globalData, offset, value);
+                putDirect(globalData, offset, value);
                 return true;
             }
             // case (2) Despecify, fall through to (3).
@@ -1375,7 +1363,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
 
         // case (3) set the slot, do the put, return.
         slot.setExistingProperty(this, offset);
-        putDirectOffset(globalData, offset, value);
+        putDirect(globalData, offset, value);
         return true;
     }
 
@@ -1388,7 +1376,7 @@ inline bool JSObject::putDirectInternal(JSGlobalData& globalData, PropertyName p
     ASSERT(structure->isValidOffset(offset));
     setStructureAndReallocateStorageIfNecessary(globalData, structure);
 
-    putDirectOffset(globalData, offset, value);
+    putDirect(globalData, offset, value);
     // This is a new property; transitions with specific values are not currently cachable,
     // so leave the slot in an uncachable state.
     if (!specificFunction)
@@ -1448,7 +1436,7 @@ inline void JSObject::putDirectWithoutTransition(JSGlobalData& globalData, Prope
         newButterfly = growOutOfLineStorage(globalData, structure()->outOfLineCapacity(), structure()->suggestedNewOutOfLineStorageCapacity());
     PropertyOffset offset = structure()->addPropertyWithoutTransition(globalData, propertyName, attributes, getCallableObject(value));
     setButterfly(globalData, newButterfly, structure());
-    putDirectOffset(globalData, offset, value);
+    putDirect(globalData, offset, value);
 }
 
 inline JSValue JSObject::toPrimitive(ExecState* exec, PreferredPrimitiveType preferredType) const

Source/JavaScriptCore/runtime/JSScope.cpp

@@ -166,7 +166,7 @@ static bool executeResolveOperations(CallFrame* callFrame, JSScope* scope, const
         case ResolveOperation::GetAndReturnGlobalProperty: {
             JSGlobalObject* globalObject = scope->globalObject();
             if (globalObject->structure() == pc->m_structure.get()) {
-                result.setValue(globalObject->getDirectOffset(pc->m_offset));
+                result.setValue(globalObject->getDirect(pc->m_offset));
                 return true;
             }
 
@@ -581,7 +581,7 @@ void JSScope::resolvePut(CallFrame* callFrame, JSValue base, const Identifier& p
         JSObject* object = jsCast<JSObject*>(base);
         if (operation->m_structure.get() != object->structure())
             break;
-        object->putDirectOffset(callFrame->globalData(), operation->m_offset, value);
+        object->putDirect(callFrame->globalData(), operation->m_offset, value);
         return;
     }
 

Source/JavaScriptCore/runtime/JSValue.cpp

@@ -144,7 +144,7 @@ void JSValue::putToPrimitive(ExecState* exec, PropertyName propertyName, JSValue
                 return;
             }
 
-            JSValue gs = obj->getDirectOffset(offset);
+            JSValue gs = obj->getDirect(offset);
             if (gs.isGetterSetter()) {
                 JSObject* setterFunc = asGetterSetter(gs)->setter();        
                 if (!setterFunc) {

Source/JavaScriptCore/runtime/Lookup.cpp

@@ -68,9 +68,9 @@ bool setUpStaticFunctionSlot(ExecState* exec, const HashEntry* entry, JSObject*
 {
     ASSERT(thisObj->globalObject());
     ASSERT(entry->attributes() & Function);
-    WriteBarrierBase<Unknown>* location = thisObj->getDirectLocation(exec->globalData(), propertyName);
+    PropertyOffset offset = thisObj->getDirectOffset(exec->globalData(), propertyName);
 
-    if (!location) {
+    if (!isValidOffset(offset)) {
         // If a property is ever deleted from an object with a static table, then we reify
         // all static functions at that time - after this we shouldn't be re-adding anything.
         if (thisObj->staticFunctionsReified())
@@ -81,10 +81,11 @@ bool setUpStaticFunctionSlot(ExecState* exec, const HashEntry* entry, JSObject*
 
         JSFunction* function = JSFunction::create(exec, thisObj->globalObject(), entry->functionLength(), name, entry->function(), entry->intrinsic());
         thisObj->putDirect(exec->globalData(), propertyName, function, entry->attributes());
-        location = thisObj->getDirectLocation(exec->globalData(), propertyName);
+        offset = thisObj->getDirectOffset(exec->globalData(), propertyName);
+        ASSERT(isValidOffset(offset));
     }
 
-    slot.setValue(thisObj, location->get(), thisObj->offsetForLocation(location));
+    slot.setValue(thisObj, thisObj->getDirect(offset), offset);
     return true;
 }
 

Source/JavaScriptCore/runtime/Structure.cpp

@@ -634,13 +634,13 @@ Structure* Structure::flattenDictionaryStructure(JSGlobalData& globalData, JSObj
         unsigned i = 0;
         PropertyTable::iterator end = m_propertyTable->end();
         for (PropertyTable::iterator iter = m_propertyTable->begin(); iter != end; ++iter, ++i) {
-            values[i] = object->getDirectOffset(iter->offset);
+            values[i] = object->getDirect(iter->offset);
             iter->offset = offsetForPropertyNumber(i, m_inlineCapacity);
         }
 
         // Copies in our values to their compacted locations.
         for (unsigned i = 0; i < propertyCount; i++)
-            object->putDirectOffset(globalData, offsetForPropertyNumber(i, m_inlineCapacity), values[i]);
+            object->putDirect(globalData, offsetForPropertyNumber(i, m_inlineCapacity), values[i]);
 
         m_propertyTable->clearDeletedOffsets();
     }