Get rid of the tmpProperty in StringObject

This was a bad hack. The new code is cleaner, and should
also perform faster in a couple of cases (avoiding the creation
of a temporary String that is then only thrown away).

Change-Id: Ia6f978e037506484adbc01a61606307d4645b343
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

4 files changed, 34 insertions, 34 deletions

diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 170585abe4..a5ec07006e 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -251,7 +251,7 @@ void Object::getOwnProperty(uint index, PropertyAttributes *attrs, Property *p)
     if (isStringObject()) {
         *attrs = Attr_NotConfigurable|Attr_NotWritable;
         if (p)
-            p->copy(static_cast<StringObject *>(this)->getIndex(index), *attrs);
+            p->value = static_cast<StringObject *>(this)->getIndex(index);
         return;
     }
 
@@ -263,10 +263,7 @@ void Object::getOwnProperty(uint index, PropertyAttributes *attrs, Property *p)
 // Section 8.12.2
 Property *Object::__getPropertyDescriptor__(String *name, PropertyAttributes *attrs) const
 {
-    uint idx = name->asArrayIndex();
-    if (idx != UINT_MAX)
-        return __getPropertyDescriptor__(idx, attrs);
-
+    Q_ASSERT(name->asArrayIndex() == UINT_MAX);
 
     const Heap::Object *o = d();
     while (o) {
@@ -290,16 +287,15 @@ Property *Object::__getPropertyDescriptor__(uint index, PropertyAttributes *attr
     while (o) {
         Property *p = o->arrayData ? o->arrayData->getProperty(index) : 0;
         if (p) {
-            if (attrs)
-                *attrs = o->arrayData->attributes(index);
+            *attrs = o->arrayData->attributes(index);
             return p;
         }
         if (o->vtable->type == Type_StringObject) {
-            Property *p = static_cast<const Heap::StringObject *>(o)->getIndex(index);
-            if (p) {
-                if (attrs)
-                    *attrs = (Attr_NotWritable|Attr_NotConfigurable);
-                return p;
+            if (index < static_cast<const Heap::StringObject *>(o)->length()) {
+                // this is an evil hack, but it works, as the method is only ever called from putIndexed,
+                // where we don't use the returned pointer there for non writable attributes
+                *attrs = (Attr_NotWritable|Attr_NotConfigurable);
+                return reinterpret_cast<Property *>(0x1);
             }
         }
         o = o->prototype;
@@ -629,10 +625,12 @@ ReturnedValue Object::internalGetIndexed(uint index, bool *hasProperty) const
             break;
         }
         if (o->isStringObject()) {
-            pd = static_cast<StringObject *>(o.getPointer())->getIndex(index);
-            if (pd) {
+            ScopedString str(scope, static_cast<StringObject *>(o.getPointer())->getIndex(index));
+            if (str) {
                 attrs = (Attr_NotWritable|Attr_NotConfigurable);
-                break;
+                if (hasProperty)
+                    *hasProperty = true;
+                return str.asReturnedValue();
             }
         }
         o = o->prototype();
@@ -749,8 +747,7 @@ void Object::internalPutIndexed(uint index, const Value &value)
         attrs = arrayData()->attributes(index);
 
     if (!pd && isStringObject()) {
-        pd = static_cast<StringObject *>(this)->getIndex(index);
-        if (pd)
+        if (index < static_cast<StringObject *>(this)->length())
             // not writable
             goto reject;
     }
@@ -930,16 +927,16 @@ reject:
 
 bool Object::defineOwnProperty2(ExecutionEngine *engine, uint index, const Property *p, PropertyAttributes attrs)
 {
-    Property *current = 0;
+    bool hasProperty = 0;
 
     // Clause 1
     if (arrayData()) {
-        current = arrayData()->getProperty(index);
-        if (!current && isStringObject())
-            current = static_cast<StringObject *>(this)->getIndex(index);
+        hasProperty = arrayData()->getProperty(index);
+        if (!hasProperty  && isStringObject())
+            hasProperty = (index < static_cast<StringObject *>(this)->length());
     }
 
-    if (!current) {
+    if (!hasProperty) {
         // clause 3
         if (!isExtensible())
             goto reject;
diff --git a/src/qml/jsruntime/qv4object_p.h b/src/qml/jsruntime/qv4object_p.h
index 6a0c38abec..671e207489 100644
--- a/src/qml/jsruntime/qv4object_p.h
+++ b/src/qml/jsruntime/qv4object_p.h
@@ -147,8 +147,8 @@ struct Q_QML_EXPORT Object: Managed {
     void getOwnProperty(String *name, PropertyAttributes *attrs, Property *p = 0);
     void getOwnProperty(uint index, PropertyAttributes *attrs, Property *p = 0);
 
-    Property *__getPropertyDescriptor__(String *name, PropertyAttributes *attrs = 0) const;
-    Property *__getPropertyDescriptor__(uint index, PropertyAttributes *attrs = 0) const;
+    Property *__getPropertyDescriptor__(String *name, PropertyAttributes *attrs) const;
+    Property *__getPropertyDescriptor__(uint index, PropertyAttributes *attrs) const;
 
     bool hasProperty(String *name) const;
     bool hasProperty(uint index) const;
diff --git a/src/qml/jsruntime/qv4stringobject.cpp b/src/qml/jsruntime/qv4stringobject.cpp
index 319768746b..08b078f8c5 100644
--- a/src/qml/jsruntime/qv4stringobject.cpp
+++ b/src/qml/jsruntime/qv4stringobject.cpp
@@ -72,7 +72,6 @@ Heap::StringObject::StringObject(InternalClass *ic, QV4::Object *prototype)
 {
     Q_ASSERT(vtable == QV4::StringObject::staticVTable());
     value = ic->engine->newString()->asReturnedValue();
-    tmpProperty.value = Primitive::undefinedValue();
 
     Scope scope(ic->engine);
     ScopedObject s(scope, this);
@@ -84,20 +83,23 @@ Heap::StringObject::StringObject(ExecutionEngine *engine, const Value &val)
 {
     value = val;
     Q_ASSERT(value.isString());
-    tmpProperty.value = Primitive::undefinedValue();
 
     Scope scope(engine);
     ScopedObject s(scope, this);
     s->defineReadonlyProperty(engine->id_length(), Primitive::fromUInt32(value.stringValue()->toQString().length()));
 }
 
-Property *Heap::StringObject::getIndex(uint index) const
+Heap::String *Heap::StringObject::getIndex(uint index) const
 {
     QString str = value.stringValue()->toQString();
     if (index >= (uint)str.length())
         return 0;
-    tmpProperty.value = Encode(internalClass->engine->newString(str.mid(index, 1)));
-    return &tmpProperty;
+    return internalClass->engine->newString(str.mid(index, 1));
+}
+
+uint Heap::StringObject::length() const
+{
+    return value.stringValue()->toQString().length();
 }
 
 bool StringObject::deleteIndexedProperty(Managed *m, uint index)
@@ -148,7 +150,6 @@ void StringObject::markObjects(Heap::Base *that, ExecutionEngine *e)
 {
     StringObject::Data *o = static_cast<StringObject::Data *>(that);
     o->value.stringValue()->mark(e);
-    o->tmpProperty.value.mark(e);
     Object::markObjects(that, e);
 }
 
diff --git a/src/qml/jsruntime/qv4stringobject_p.h b/src/qml/jsruntime/qv4stringobject_p.h
index b0f2dd3e97..8b05cfd3dc 100644
--- a/src/qml/jsruntime/qv4stringobject_p.h
+++ b/src/qml/jsruntime/qv4stringobject_p.h
@@ -48,9 +48,8 @@ struct StringObject : Object {
     StringObject(ExecutionEngine *engine, const Value &value);
     Value value;
 
-    Property *getIndex(uint index) const;
-    // ### get rid of tmpProperty
-    mutable Property tmpProperty;
+    Heap::String *getIndex(uint index) const;
+    uint length() const;
 };
 
 struct StringCtor : FunctionObject {
@@ -63,9 +62,12 @@ struct StringObject: Object {
     V4_OBJECT2(StringObject, Object)
     Q_MANAGED_TYPE(StringObject)
 
-    Property *getIndex(uint index) const {
+    Heap::String *getIndex(uint index) const {
         return d()->getIndex(index);
     }
+    uint length() const {
+        return d()->length();
+    }
 
     static bool deleteIndexedProperty(Managed *m, uint index);