Cleanup API of Safe<T>

Don't have an implicit cast operator to Returned<T>
anymore, and return a T* from the operator->()

Change-Id: If4165071b986bfc84a157560d94d39c2dcfbc9e1
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

12 files changed, 58 insertions, 33 deletions

diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp
index 674dd46e96..0e08f025c2 100644
--- a/src/qml/jsruntime/qv4context.cpp
+++ b/src/qml/jsruntime/qv4context.cpp
@@ -286,7 +286,7 @@ bool ExecutionContext::deleteProperty(const StringRef name)
                 return w->withObject->deleteProperty(name);
         } else if (ctx->type == Type_CatchContext) {
             CatchContext *c = static_cast<CatchContext *>(ctx);
-            if (c->exceptionVarName->stringValue()->isEqualTo(name))
+            if (c->exceptionVarName->isEqualTo(name))
                 return false;
         } else if (ctx->type >= Type_CallContext) {
             CallContext *c = static_cast<CallContext *>(ctx);
@@ -361,7 +361,7 @@ void ExecutionContext::setProperty(const StringRef name, const ValueRef value)
                 w->put(name, value);
                 return;
             }
-        } else if (ctx->type == Type_CatchContext && static_cast<CatchContext *>(ctx)->exceptionVarName->stringValue()->isEqualTo(name)) {
+        } else if (ctx->type == Type_CatchContext && static_cast<CatchContext *>(ctx)->exceptionVarName->isEqualTo(name)) {
             static_cast<CatchContext *>(ctx)->exceptionValue = *value;
             return;
         } else {
@@ -389,7 +389,7 @@ void ExecutionContext::setProperty(const StringRef name, const ValueRef value)
             }
         }
     }
-    if (strictMode || name->isEqualTo(engine->id_this)) {
+    if (strictMode || name->equals(engine->id_this)) {
         ScopedValue n(scope, name.asReturnedValue());
         throwReferenceError(n);
     }
@@ -402,7 +402,7 @@ ReturnedValue ExecutionContext::getProperty(const StringRef name)
     ScopedValue v(scope);
     name->makeIdentifier();
 
-    if (name->isEqualTo(engine->id_this))
+    if (name->equals(engine->id_this))
         return callData->thisObject.asReturnedValue();
 
     bool hasWith = false;
@@ -422,7 +422,7 @@ ReturnedValue ExecutionContext::getProperty(const StringRef name)
         else if (ctx->type == Type_CatchContext) {
             hasCatchScope = true;
             CatchContext *c = static_cast<CatchContext *>(ctx);
-            if (c->exceptionVarName->stringValue()->isEqualTo(name))
+            if (c->exceptionVarName->isEqualTo(name))
                 return c->exceptionValue.asReturnedValue();
         }
 
@@ -444,7 +444,7 @@ ReturnedValue ExecutionContext::getProperty(const StringRef name)
                     return v.asReturnedValue();
             }
             if (f->function && f->function->isNamedExpression()
-                && name->isEqualTo(f->function->name))
+                && name->equals(f->function->name))
                 return f.asReturnedValue();
         }
 
@@ -467,7 +467,7 @@ ReturnedValue ExecutionContext::getPropertyNoThrow(const StringRef name)
     ScopedValue v(scope);
     name->makeIdentifier();
 
-    if (name->isEqualTo(engine->id_this))
+    if (name->equals(engine->id_this))
         return callData->thisObject.asReturnedValue();
 
     bool hasWith = false;
@@ -487,7 +487,7 @@ ReturnedValue ExecutionContext::getPropertyNoThrow(const StringRef name)
         else if (ctx->type == Type_CatchContext) {
             hasCatchScope = true;
             CatchContext *c = static_cast<CatchContext *>(ctx);
-            if (c->exceptionVarName->stringValue()->isEqualTo(name))
+            if (c->exceptionVarName->isEqualTo(name))
                 return c->exceptionValue.asReturnedValue();
         }
 
@@ -509,7 +509,7 @@ ReturnedValue ExecutionContext::getPropertyNoThrow(const StringRef name)
                     return v.asReturnedValue();
             }
             if (f->function && f->function->isNamedExpression()
-                && name->isEqualTo(f->function->name))
+                && name->equals(f->function->name))
                 return f.asReturnedValue();
         }
 
@@ -531,7 +531,7 @@ ReturnedValue ExecutionContext::getPropertyAndBase(const StringRef name, ObjectR
     base = (Object *)0;
     name->makeIdentifier();
 
-    if (name->isEqualTo(engine->id_this))
+    if (name->equals(engine->id_this))
         return callData->thisObject.asReturnedValue();
 
     bool hasWith = false;
@@ -552,7 +552,7 @@ ReturnedValue ExecutionContext::getPropertyAndBase(const StringRef name, ObjectR
         else if (ctx->type == Type_CatchContext) {
             hasCatchScope = true;
             CatchContext *c = static_cast<CatchContext *>(ctx);
-            if (c->exceptionVarName->stringValue()->isEqualTo(name))
+            if (c->exceptionVarName->isEqualTo(name))
                 return c->exceptionValue.asReturnedValue();
         }
 
@@ -577,7 +577,7 @@ ReturnedValue ExecutionContext::getPropertyAndBase(const StringRef name, ObjectR
                 }
             }
             if (f->function && f->function->isNamedExpression()
-                && name->isEqualTo(f->function->name))
+                && name->equals(f->function->name))
                 return c->function->asReturnedValue();
         }
 
diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index 4f6adc2c88..02783b79a0 100644
--- a/src/qml/jsruntime/qv4internalclass.cpp
+++ b/src/qml/jsruntime/qv4internalclass.cpp
@@ -197,6 +197,11 @@ InternalClass *InternalClass::changePrototype(Object *proto)
     return newClass;
 }
 
+InternalClass *InternalClass::addMember(StringRef string, PropertyAttributes data, uint *index)
+{
+    return addMember(string.getPointer(), data, index);
+}
+
 InternalClass *InternalClass::addMember(String *string, PropertyAttributes data, uint *index)
 {
 //    qDebug() << "InternalClass::addMember()" << string->toQString() << size << hex << (uint)data.m_all << data.type();
@@ -255,6 +260,11 @@ void InternalClass::removeMember(Object *object, Identifier *id)
     transitions.insert(t, object->internalClass);
 }
 
+uint InternalClass::find(const StringRef string)
+{
+    return find(string.getPointer());
+}
+
 uint InternalClass::find(const String *string)
 {
     engine->identifierTable->identifier(string);
diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index ecb5c0bfab..659789b344 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -137,9 +137,11 @@ struct InternalClass {
     uint size;
 
     InternalClass *changePrototype(Object *proto);
+    InternalClass *addMember(StringRef string, PropertyAttributes data, uint *index = 0);
     InternalClass *addMember(String *string, PropertyAttributes data, uint *index = 0);
     InternalClass *changeMember(String *string, PropertyAttributes data, uint *index = 0);
     void removeMember(Object *object, Identifier *id);
+    uint find(const StringRef string);
     uint find(const String *s);
 
     InternalClass *sealed();
diff --git a/src/qml/jsruntime/qv4lookup.cpp b/src/qml/jsruntime/qv4lookup.cpp
index 4527dcb33c..4ee24d29eb 100644
--- a/src/qml/jsruntime/qv4lookup.cpp
+++ b/src/qml/jsruntime/qv4lookup.cpp
@@ -104,7 +104,7 @@ ReturnedValue Lookup::getterGeneric(QV4::Lookup *l, const ValueRef object)
     case Value::Managed_Type:
         Q_ASSERT(object->isString());
         proto = engine->stringClass->prototype;
-        if (l->name == engine->id_length) {
+        if (l->name->equals(engine->id_length)) {
             // special case, as the property is on the object itself
             l->getter = stringLengthGetter;
             return stringLengthGetter(l, object);
diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index eaa3b592f0..a07e5dfe1d 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -693,7 +693,7 @@ void Object::internalPut(const StringRef name, const ValueRef value)
             goto reject;
         } else if (!attrs.isWritable())
             goto reject;
-        else if (isArrayObject() && name->isEqualTo(engine()->id_length)) {
+        else if (isArrayObject() && name->equals(engine()->id_length)) {
             bool ok;
             uint l = value->asArrayLength(&ok);
             if (!ok)
@@ -885,7 +885,7 @@ bool Object::__defineOwnProperty__(ExecutionContext *ctx, const StringRef name,
     Property *current;
     PropertyAttributes *cattrs;
 
-    if (isArrayObject() && name->isEqualTo(ctx->engine->id_length)) {
+    if (isArrayObject() && name->equals(ctx->engine->id_length)) {
         assert(ArrayObject::LengthPropertyIndex == internalClass->find(ctx->engine->id_length));
         Property *lp = memberData + ArrayObject::LengthPropertyIndex;
         cattrs = internalClass->propertyData.data() + ArrayObject::LengthPropertyIndex;
diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 92cfdd8301..8be1343ac5 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -244,7 +244,6 @@ QObjectWrapper::QObjectWrapper(ExecutionEngine *engine, QObject *object)
     vtbl = &static_vtbl;
 
     m_destroy = engine->newIdentifier(QStringLiteral("destroy"));
-    m_toString = engine->id_toString;
 }
 
 void QObjectWrapper::initializeBindings(ExecutionEngine *engine)
@@ -278,8 +277,8 @@ ReturnedValue QObjectWrapper::getQmlProperty(ExecutionContext *ctx, QQmlContextD
     QV4:Scope scope(ctx);
     QV4::ScopedString name(scope, n);
 
-    if (name->isEqualTo(m_destroy) || name->isEqualTo(m_toString)) {
-        int index = name->isEqualTo(m_destroy) ? QV4::QObjectMethod::DestroyMethod : QV4::QObjectMethod::ToStringMethod;
+    if (name->equals(m_destroy) || name->equals(scope.engine->id_toString)) {
+        int index = name->equals(m_destroy) ? QV4::QObjectMethod::DestroyMethod : QV4::QObjectMethod::ToStringMethod;
         QV4::ScopedValue method(scope, QV4::QObjectMethod::create(ctx->engine->rootContext, m_object, index));
         if (hasProperty)
             *hasProperty = true;
@@ -640,7 +639,7 @@ PropertyAttributes QObjectWrapper::query(const Managed *m, StringRef name)
     QQmlContextData *qmlContext = QV4::QmlContextWrapper::callingContext(engine);
     QQmlPropertyData local;
     if (that->findProperty(engine, qmlContext, name, IgnoreRevision, &local)
-        || name->isEqualTo(that->m_destroy) || name->isEqualTo(that->m_toString))
+        || name->equals(const_cast<SafeString &>(that->m_destroy)) || name->equals(engine->id_toString))
         return QV4::Attr_Data;
     else
         return QV4::Object::query(m, name);
diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h
index 7b9e985f21..eadbacc096 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper_p.h
+++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h
@@ -102,8 +102,7 @@ private:
     QQmlPropertyData *findProperty(ExecutionEngine *engine, QQmlContextData *qmlContext, String *name, RevisionMode revisionMode, QQmlPropertyData *local) const;
 
     QPointer<QObject> m_object;
-    String *m_destroy;
-    String *m_toString;
+    SafeString m_destroy;
 
     static ReturnedValue get(Managed *m, const StringRef name, bool *hasProperty);
     static void put(Managed *m, const StringRef name, const ValueRef value);
diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp
index 95fe39ba5a..6002346678 100644
--- a/src/qml/jsruntime/qv4runtime.cpp
+++ b/src/qml/jsruntime/qv4runtime.cpp
@@ -424,14 +424,14 @@ Returned<String> *__qmljs_convert_to_string(ExecutionContext *ctx, const ValueRe
     case Value::Empty_Type:
         Q_ASSERT(!"empty Value encountered");
     case Value::Undefined_Type:
-        return ctx->engine->id_undefined;
+        return ctx->engine->id_undefined.ret();
     case Value::Null_Type:
-        return ctx->engine->id_null;
+        return ctx->engine->id_null.ret();
     case Value::Boolean_Type:
         if (value->booleanValue())
-            return ctx->engine->id_true;
+            return ctx->engine->id_true.ret();
         else
-            return ctx->engine->id_false;
+            return ctx->engine->id_false.ret();
     case Value::Managed_Type:
         if (value->isString())
             return value->stringValue()->asReturned<String>();
@@ -737,7 +737,7 @@ ReturnedValue __qmljs_call_global_lookup(ExecutionContext *context, uint index,
     if (!o)
         context->throwTypeError();
 
-    if (o.getPointer() == context->engine->evalFunction && l->name->isEqualTo(context->engine->id_eval))
+    if (o.getPointer() == context->engine->evalFunction && l->name->equals(context->engine->id_eval))
         return static_cast<EvalFunction *>(o.getPointer())->evalCall(callData, true);
 
     return o->call(callData);
@@ -763,7 +763,7 @@ ReturnedValue __qmljs_call_activation_property(ExecutionContext *context, const
         context->throwTypeError(msg);
     }
 
-    if (o == context->engine->evalFunction && name->isEqualTo(context->engine->id_eval)) {
+    if (o == context->engine->evalFunction && name->equals(context->engine->id_eval)) {
         return static_cast<EvalFunction *>(o)->evalCall(callData, true);
     }
 
diff --git a/src/qml/jsruntime/qv4scopedvalue_p.h b/src/qml/jsruntime/qv4scopedvalue_p.h
index 2e39f035fe..2e76eb453e 100644
--- a/src/qml/jsruntime/qv4scopedvalue_p.h
+++ b/src/qml/jsruntime/qv4scopedvalue_p.h
@@ -687,7 +687,7 @@ inline Safe<T> &Safe<T>::operator=(const Safe<T> &t)
 }
 
 template<typename T>
-inline Safe<T>::operator Returned<T> *()
+inline Returned<T> * Safe<T>::ret() const
 {
     return Returned<T>::create(static_cast<T *>(managed()));
 }
diff --git a/src/qml/jsruntime/qv4string.cpp b/src/qml/jsruntime/qv4string.cpp
index af573fb471..fd366d26ac 100644
--- a/src/qml/jsruntime/qv4string.cpp
+++ b/src/qml/jsruntime/qv4string.cpp
@@ -135,7 +135,7 @@ ReturnedValue String::get(Managed *m, const StringRef name, bool *hasProperty)
     Scope scope(v4);
     ScopedString that(scope, static_cast<String *>(m));
 
-    if (name->isEqualTo(v4->id_length)) {
+    if (name->equals(v4->id_length)) {
         if (hasProperty)
             *hasProperty = true;
         return Primitive::fromInt32(that->_text.length()).asReturnedValue();
@@ -260,6 +260,20 @@ uint String::toUInt(bool *ok) const
     return UINT_MAX;
 }
 
+bool String::equals(const StringRef other) const
+{
+    if (this == other.getPointer())
+        return true;
+    if (hashValue() != other->hashValue())
+        return false;
+    if (identifier && identifier == other->identifier)
+        return true;
+    if (subtype >= StringType_UInt && subtype == other->subtype)
+        return true;
+
+    return toQString() == other->toQString();
+}
+
 void String::makeIdentifierImpl() const
 {
     engine()->identifierTable->identifier(this);
diff --git a/src/qml/jsruntime/qv4string_p.h b/src/qml/jsruntime/qv4string_p.h
index b194003005..1e2aba32a9 100644
--- a/src/qml/jsruntime/qv4string_p.h
+++ b/src/qml/jsruntime/qv4string_p.h
@@ -65,6 +65,7 @@ struct Q_QML_EXPORT String : public Managed {
     String(ExecutionEngine *engine, const QString &text);
     ~String() { _data = 0; }
 
+    bool equals(const StringRef other) const;
     inline bool isEqualTo(const String *other) const {
         if (this == other)
             return true;
diff --git a/src/qml/jsruntime/qv4value_def_p.h b/src/qml/jsruntime/qv4value_def_p.h
index f552798a4b..c0b59f6711 100644
--- a/src/qml/jsruntime/qv4value_def_p.h
+++ b/src/qml/jsruntime/qv4value_def_p.h
@@ -400,10 +400,10 @@ struct Safe : public SafeValue
 
     bool operator!() const { return !managed(); }
 
-    // ### GC: remove me
-    operator T*() { return static_cast<T *>(managed()); }
-    Value *operator->() { return this; }
-    operator Returned<T> *();
+    T *operator->() { return static_cast<T *>(managed()); }
+    const T *operator->() const { return static_cast<T *>(managed()); }
+    T *getPointer() const { return static_cast<T *>(managed()); }
+    Returned<T> *ret() const;
 };
 typedef Safe<String> SafeString;
 typedef Safe<Object> SafeObject;