Refactor InternalClass::find()

Specialize find() into several methods for different purposes.
Prepares for further cleanups and being able to split up
getter and setter for accessor properties.

Change-Id: Id4ec5509ac1a1361e2170bbfc2347b89b520c782
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

10 files changed, 103 insertions, 64 deletions

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index 50672445d1..9cebe6f043 100644
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -58,8 +58,8 @@ void Heap::StrictArgumentsObject::init(QV4::CppStackFrame *frame)
 
     Object::init();
 
-    Q_ASSERT(CalleePropertyIndex == internalClass->find(v4->id_callee()->propertyKey()));
-    Q_ASSERT(SymbolIteratorPropertyIndex == internalClass->find(v4->symbol_iterator()->propertyKey()));
+    Q_ASSERT(internalClass->verifyIndex(v4->id_callee()->propertyKey(), CalleePropertyIndex));
+    Q_ASSERT(internalClass->verifyIndex(v4->symbol_iterator()->propertyKey(), SymbolIteratorPropertyIndex));
     setProperty(v4, SymbolIteratorPropertyIndex, *v4->arrayProtoValues());
     setProperty(v4, CalleePropertyIndex + QV4::Object::GetterOffset, *v4->thrower());
     setProperty(v4, CalleePropertyIndex + QV4::Object::SetterOffset, *v4->thrower());
@@ -69,7 +69,7 @@ void Heap::StrictArgumentsObject::init(QV4::CppStackFrame *frame)
     args->arrayReserve(frame->originalArgumentsCount);
     args->arrayPut(0, frame->originalArguments, frame->originalArgumentsCount);
 
-    Q_ASSERT(LengthPropertyIndex == args->internalClass()->find(v4->id_length()->propertyKey()));
+    Q_ASSERT(args->internalClass()->verifyIndex(v4->id_length()->propertyKey(), LengthPropertyIndex));
     setProperty(v4, LengthPropertyIndex, Value::fromInt32(frame->originalArgumentsCount));
 }
 
@@ -83,11 +83,11 @@ void Heap::ArgumentsObject::init(QV4::CppStackFrame *frame)
     this->context.set(v4, context->d());
     Q_ASSERT(vtable() == QV4::ArgumentsObject::staticVTable());
 
-    Q_ASSERT(CalleePropertyIndex == internalClass->find(v4->id_callee()->propertyKey()));
+    Q_ASSERT(internalClass->verifyIndex(v4->id_callee()->propertyKey(), CalleePropertyIndex));
     setProperty(v4, CalleePropertyIndex, context->d()->function);
-    Q_ASSERT(LengthPropertyIndex == internalClass->find(v4->id_length()->propertyKey()));
+    Q_ASSERT(internalClass->verifyIndex(v4->id_length()->propertyKey(), LengthPropertyIndex));
     setProperty(v4, LengthPropertyIndex, Value::fromInt32(context->argc()));
-    Q_ASSERT(SymbolIteratorPropertyIndex == internalClass->find(v4->symbol_iterator()->propertyKey()));
+    Q_ASSERT(internalClass->verifyIndex(v4->symbol_iterator()->propertyKey(), SymbolIteratorPropertyIndex));
     setProperty(v4, SymbolIteratorPropertyIndex, *v4->arrayProtoValues());
 
     fullyCreated = false;
diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp
index 6e3e7f9b4f..38e233965f 100644
--- a/src/qml/jsruntime/qv4context.cpp
+++ b/src/qml/jsruntime/qv4context.cpp
@@ -220,7 +220,7 @@ bool ExecutionContext::deleteProperty(String *name)
         case Heap::ExecutionContext::Type_BlockContext:
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
-            uint index = c->internalClass->find(id);
+            uint index = c->internalClass->indexOfValueOrGetter(id);
             if (index < UINT_MAX)
                 // ### throw in strict mode?
                 return false;
@@ -286,7 +286,7 @@ ExecutionContext::Error ExecutionContext::setProperty(String *name, const Value
         case Heap::ExecutionContext::Type_BlockContext:
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
-            uint index = c->internalClass->find(id);
+            uint index = c->internalClass->indexOfValueOrGetter(id);
             if (index < UINT_MAX) {
                 static_cast<Heap::CallContext *>(c)->locals.set(engine, index, value);
                 return NoError;
@@ -295,7 +295,7 @@ ExecutionContext::Error ExecutionContext::setProperty(String *name, const Value
             Q_FALLTHROUGH();
         case Heap::ExecutionContext::Type_GlobalContext:
             if (ctx->activation) {
-                uint member = ctx->activation->internalClass->find(id);
+                uint member = ctx->activation->internalClass->indexOfValueOrGetter(id);
                 if (member < UINT_MAX) {
                     Scope scope(engine);
                     ScopedObject a(scope, ctx->activation);
@@ -332,7 +332,7 @@ ReturnedValue ExecutionContext::getProperty(String *name)
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
 
-            uint index = c->internalClass->find(id);
+            uint index = c->internalClass->indexOfValueOrGetter(id);
             if (index < UINT_MAX)
                 return c->locals[index].asReturnedValue();
             Q_FALLTHROUGH();
@@ -382,7 +382,7 @@ ReturnedValue ExecutionContext::getPropertyAndBase(String *name, Value *base)
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
 
-            uint index = c->internalClass->find(id);
+            uint index = c->internalClass->indexOfValueOrGetter(id);
             if (index < UINT_MAX)
                 return c->locals[index].asReturnedValue();
             Q_FALLTHROUGH();
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index bf8a91f6e1..dc5f80df7b 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -333,7 +333,7 @@ ExecutionEngine::ExecutionEngine(QJSEngine *jsEngine)
     ic = newInternalClass(QV4::StringObject::staticVTable(), objectPrototype());
     ic = ic->addMember(id_length()->propertyKey(), Attr_ReadOnly);
     classes[Class_StringObject] = ic->changePrototype(stringPrototype()->d());
-    Q_ASSERT(classes[Class_StringObject]->find(id_length()->propertyKey()) == Heap::StringObject::LengthPropertyIndex);
+    Q_ASSERT(classes[Class_StringObject]->verifyIndex(id_length()->propertyKey(), Heap::StringObject::LengthPropertyIndex));
 
     classes[Class_SymbolObject] = newInternalClass(QV4::SymbolObject::staticVTable(), symbolPrototype());
 
diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index 6fc2d45630..cc5f086276 100644
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -149,7 +149,7 @@ void FunctionObject::createDefaultPrototypeProperty(uint protoConstructorSlot)
 {
     Scope s(this);
 
-    Q_ASSERT(s.engine->internalClasses(EngineBase::Class_ObjectProto)->find(s.engine->id_constructor()->propertyKey()) == protoConstructorSlot);
+    Q_ASSERT(s.engine->internalClasses(EngineBase::Class_ObjectProto)->verifyIndex(s.engine->id_constructor()->propertyKey(), protoConstructorSlot));
 
     ScopedObject proto(s, s.engine->newObject(s.engine->internalClasses(EngineBase::Class_ObjectProto)));
     proto->setProperty(protoConstructorSlot, d());
@@ -538,7 +538,7 @@ void Heap::ArrowFunction::init(QV4::ExecutionContext *scope, Function *function,
     if (name)
         f->setName(name);
 
-    Q_ASSERT(internalClass && internalClass->find(s.engine->id_length()->propertyKey()) == Index_Length);
+    Q_ASSERT(internalClass && internalClass->verifyIndex(s.engine->id_length()->propertyKey(), Index_Length));
     setProperty(s.engine, Index_Length, Value::fromInt32(int(function->compiledFunction->length)));
 }
 
diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index 602061bb6e..bd037ef8a6 100644
--- a/src/qml/jsruntime/qv4internalclass.cpp
+++ b/src/qml/jsruntime/qv4internalclass.cpp
@@ -343,8 +343,9 @@ static void addDummyEntry(InternalClass *newClass, PropertyHash::Entry e)
 Heap::InternalClass *InternalClass::changeMember(PropertyKey identifier, PropertyAttributes data, uint *index)
 {
     data.resolve();
-    uint idx = find(identifier);
-    Q_ASSERT(idx != UINT_MAX);
+    PropertyHash::Entry *e = findEntry(identifier);
+    Q_ASSERT(e && e->index != UINT_MAX);
+    uint idx = e->index;
 
     if (index)
         *index = idx;
diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index 58aca42d6e..82ace19d30 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -352,8 +352,35 @@ struct InternalClass : Base {
     Q_REQUIRED_RESULT InternalClass *changeMember(PropertyKey identifier, PropertyAttributes data, uint *index = nullptr);
     static void changeMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, uint *index = nullptr);
     static void removeMember(QV4::Object *object, PropertyKey identifier);
+    PropertyHash::Entry *findEntry(const PropertyKey id)
+    {
+        Q_ASSERT(id.isStringOrSymbol());
+
+        PropertyHash::Entry *e = propertyTable.lookup(id);
+        if (e && e->index < size)
+            return e;
+
+        return nullptr;
+    }
+
+    struct IndexAndAttribute {
+        uint index;
+        PropertyAttributes attrs;
+        bool isValid() const { return index != UINT_MAX; }
+    };
+
+    IndexAndAttribute find(const PropertyKey id)
+    {
+        Q_ASSERT(id.isStringOrSymbol());
 
-    uint find(const PropertyKey id)
+        PropertyHash::Entry *e = propertyTable.lookup(id);
+        if (e && e->index < size)
+            return { e->index, propertyData.at(e->index) };
+
+        return { UINT_MAX, Attr_Invalid };
+    }
+
+    uint indexOfValueOrGetter(const PropertyKey id)
     {
         Q_ASSERT(id.isStringOrSymbol());
 
@@ -364,6 +391,17 @@ struct InternalClass : Base {
         return UINT_MAX;
     }
 
+    bool verifyIndex(const PropertyKey id, uint index)
+    {
+        Q_ASSERT(id.isStringOrSymbol());
+
+        PropertyHash::Entry *e = propertyTable.lookup(id);
+        if (e && e->index < size)
+            return e->index == index;
+
+        return false;
+    }
+
     Q_REQUIRED_RESULT InternalClass *sealed();
     Q_REQUIRED_RESULT InternalClass *frozen();
     Q_REQUIRED_RESULT InternalClass *propertiesFrozen() const;
diff --git a/src/qml/jsruntime/qv4lookup.cpp b/src/qml/jsruntime/qv4lookup.cpp
index 3875001bb0..141d9b5afb 100644
--- a/src/qml/jsruntime/qv4lookup.cpp
+++ b/src/qml/jsruntime/qv4lookup.cpp
@@ -50,10 +50,10 @@ using namespace QV4;
 void Lookup::resolveProtoGetter(PropertyKey name, const Heap::Object *proto)
 {
     while (proto) {
-        uint index = proto->internalClass->find(name);
-        if (index != UINT_MAX) {
-            PropertyAttributes attrs = proto->internalClass->propertyData.at(index);
-            protoLookup.data = proto->propertyData(index);
+        auto index = proto->internalClass->find(name);
+        if (index.isValid()) {
+            PropertyAttributes attrs = index.attrs;
+            protoLookup.data = proto->propertyData(index.index);
             if (attrs.isData()) {
                 getter = getterProto;
             } else {
@@ -77,23 +77,23 @@ ReturnedValue Lookup::resolveGetter(ExecutionEngine *engine, const Object *objec
         return getter(this, engine, *object);
     }
 
-    uint index = obj->internalClass->find(name);
-    if (index != UINT_MAX) {
-        PropertyAttributes attrs = obj->internalClass->propertyData.at(index);
+    auto index = obj->internalClass->find(name);
+    if (index.isValid()) {
+        PropertyAttributes attrs = index.attrs;
         uint nInline = obj->vtable()->nInlineProperties;
         if (attrs.isData()) {
-            if (index < obj->vtable()->nInlineProperties) {
-                index += obj->vtable()->inlinePropertyOffset;
+            if (index.index < obj->vtable()->nInlineProperties) {
+                index.index += obj->vtable()->inlinePropertyOffset;
                 getter = getter0Inline;
             } else {
-                index -= nInline;
+                index.index -= nInline;
                 getter = getter0MemberData;
             }
         } else {
             getter = getterAccessor;
         }
         objectLookup.ic = obj->internalClass;
-        objectLookup.offset = index;
+        objectLookup.offset = index.index;
         return getter(this, engine, *object);
     }
 
@@ -478,15 +478,15 @@ bool Lookup::resolveSetter(ExecutionEngine *engine, Object *object, const Value
 
     Heap::InternalClass *c = object->internalClass();
     PropertyKey key = name->toPropertyKey();
-    uint idx = c->find(key);
-    if (idx != UINT_MAX) {
-        if (object->isArrayObject() && idx == Heap::ArrayObject::LengthPropertyIndex) {
+    auto idx = c->find(key);
+    if (idx.isValid()) {
+        if (object->isArrayObject() && idx.index == Heap::ArrayObject::LengthPropertyIndex) {
             setter = arrayLengthSetter;
             return setter(this, engine, *object, value);
-        } else if (object->internalClass()->propertyData[idx].isData() && object->internalClass()->propertyData[idx].isWritable()) {
+        } else if (idx.attrs.isData() && idx.attrs.isWritable()) {
             objectLookup.ic = object->internalClass();
-            objectLookup.offset = idx;
-            setter = idx < object->d()->vtable()->nInlineProperties ? Lookup::setter0Inline : Lookup::setter0;
+            objectLookup.offset = idx.index;
+            setter = idx.index < object->d()->vtable()->nInlineProperties ? Lookup::setter0Inline : Lookup::setter0;
             return setter(this, engine, *object, value);
         } else {
             // ### handle setter
@@ -507,12 +507,12 @@ bool Lookup::resolveSetter(ExecutionEngine *engine, Object *object, const Value
         return true;
     }
     idx = object->internalClass()->find(key);
-    if (idx == UINT_MAX) { // ### can this even happen?
+    if (!idx.isValid()) { // ### can this even happen?
         setter = setterFallback;
         return false;
     }
     insertionLookup.newClass = object->internalClass();
-    insertionLookup.offset = idx;
+    insertionLookup.offset = idx.index;
     setter = setterInsert;
     return true;
 }
diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp
index 29ccecf1f2..476ecf7f2a 100644
--- a/src/qml/jsruntime/qv4object.cpp
+++ b/src/qml/jsruntime/qv4object.cpp
@@ -295,10 +295,10 @@ PropertyIndex Object::getValueOrSetter(PropertyKey id, PropertyAttributes *attrs
     } else {
         Heap::Object *o = d();
         while (o) {
-            uint idx = o->internalClass->find(id);
-            if (idx < UINT_MAX) {
-                *attrs = o->internalClass->propertyData[idx];
-                return o->writablePropertyData(attrs->isAccessor() ? idx + SetterOffset : idx );
+            auto idx = o->internalClass->find(id);
+            if (idx.isValid()) {
+                *attrs = idx.attrs;
+                return o->writablePropertyData(attrs->isAccessor() ? idx.index + SetterOffset : idx.index );
             }
 
             o = o->prototype();
@@ -443,11 +443,11 @@ ReturnedValue Object::internalGet(PropertyKey id, const Value *receiver, bool *h
         Q_ASSERT(!id.isArrayIndex());
 
         while (1) {
-            uint idx = o->internalClass->find(id);
-            if (idx < UINT_MAX) {
+            auto idx = o->internalClass->find(id);
+            if (idx.isValid()) {
                 if (hasProperty)
                     *hasProperty = true;
-                return Object::getValue(*receiver, *o->propertyData(idx), o->internalClass->propertyData.at(idx));
+                return Object::getValue(*receiver, *o->propertyData(idx.index), idx.attrs);
             }
             o = o->prototype();
             if (!o || o->internalClass->vtable->get != Object::virtualGet)
@@ -488,10 +488,10 @@ bool Object::internalPut(PropertyKey id, const Value &value, Value *receiver)
                 if (arrayData())
                     propertyIndex = arrayData()->getValueOrSetter(index, &attrs);
             } else {
-                uint member = internalClass()->find(id);
-                if (member < UINT_MAX) {
-                    attrs = internalClass()->propertyData[member];
-                    propertyIndex = d()->writablePropertyData(attrs.isAccessor() ? member + SetterOffset : member);
+                auto member = internalClass()->find(id);
+                if (member.isValid()) {
+                    attrs = member.attrs;
+                    propertyIndex = d()->writablePropertyData(member.attrs.isAccessor() ? member.index + SetterOffset : member.index);
                 }
             }
 
@@ -588,9 +588,9 @@ bool Object::internalDeleteProperty(PropertyKey id)
         return false;
     }
 
-    uint memberIdx = internalClass()->find(id);
-    if (memberIdx != UINT_MAX) {
-        if (internalClass()->propertyData[memberIdx].isConfigurable()) {
+    auto memberIdx = internalClass()->find(id);
+    if (memberIdx.isValid()) {
+        if (memberIdx.attrs.isConfigurable()) {
             Heap::InternalClass::removeMember(this, id);
             return true;
         }
@@ -804,13 +804,13 @@ PropertyAttributes Object::virtualGetOwnProperty(const Managed *m, PropertyKey i
     } else {
         Q_ASSERT(id.asStringOrSymbol());
 
-        uint member = o->internalClass()->find(id);
-        if (member < UINT_MAX) {
-            attrs = o->internalClass()->propertyData[member];
+        auto member = o->internalClass()->find(id);
+        if (member.isValid()) {
+            attrs = member.attrs;
             if (p) {
-                p->value = *o->propertyData(member);
+                p->value = *o->propertyData(member.index);
                 if (attrs.isAccessor())
-                    p->set = *o->propertyData(member + SetterOffset);
+                    p->set = *o->propertyData(member.index + SetterOffset);
             }
             return attrs;
         }
@@ -854,10 +854,10 @@ bool Object::virtualDefineOwnProperty(Managed *m, PropertyKey id, const Property
         return o->internalDefineOwnProperty(scope.engine, index, nullptr, p, attrs);
     }
 
-    uint memberIndex = o->internalClass()->find(id);
+    auto memberIndex = o->internalClass()->find(id);
     Scoped<StringOrSymbol> name(scope, id.asStringOrSymbol());
 
-    if (memberIndex == UINT_MAX) {
+    if (!memberIndex.isValid()) {
         if (!o->isExtensible())
             return false;
 
@@ -868,7 +868,7 @@ bool Object::virtualDefineOwnProperty(Managed *m, PropertyKey id, const Property
         return true;
     }
 
-    return o->internalDefineOwnProperty(scope.engine, memberIndex, name, p, attrs);
+    return o->internalDefineOwnProperty(scope.engine, memberIndex.index, name, p, attrs);
 }
 
 bool Object::virtualIsExtensible(const Managed *m)
@@ -1057,7 +1057,7 @@ bool ArrayObject::virtualDefineOwnProperty(Managed *m, PropertyKey id, const Pro
     ExecutionEngine *engine = m->engine();
     if (id == engine->id_length()->propertyKey()) {
         Scope scope(engine);
-        Q_ASSERT(Heap::ArrayObject::LengthPropertyIndex == a->internalClass()->find(engine->id_length()->propertyKey()));
+        Q_ASSERT(a->internalClass()->verifyIndex(engine->id_length()->propertyKey(), Heap::ArrayObject::LengthPropertyIndex));
         ScopedProperty lp(scope);
         PropertyAttributes cattrs;
         a->getProperty(Heap::ArrayObject::LengthPropertyIndex, lp, &cattrs);
diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index b69081d378..b10065d308 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -265,9 +265,9 @@ bool QQmlContextWrapper::virtualPut(Managed *m, PropertyKey id, const Value &val
         return false;
     QV4::Scoped<QQmlContextWrapper> wrapper(scope, resource);
 
-    uint member = wrapper->internalClass()->find(id);
-    if (member < UINT_MAX)
-        return wrapper->putValue(member, value);
+    auto member = wrapper->internalClass()->find(id);
+    if (member.index < UINT_MAX)
+        return wrapper->putValue(member.index, value);
 
     // It's possible we could delay the calculation of the "actual" context (in the case
     // of sub contexts) until it is definitely needed.
diff --git a/src/qml/jsruntime/qv4regexpobject_p.h b/src/qml/jsruntime/qv4regexpobject_p.h
index 5dfb9d9730..a584404c0b 100644
--- a/src/qml/jsruntime/qv4regexpobject_p.h
+++ b/src/qml/jsruntime/qv4regexpobject_p.h
@@ -125,11 +125,11 @@ struct RegExpObject: Object {
     void initProperties();
 
     int lastIndex() const {
-        Q_ASSERT(Index_LastIndex == internalClass()->find(engine()->id_lastIndex()->propertyKey()));
+        Q_ASSERT(internalClass()->verifyIndex(engine()->id_lastIndex()->propertyKey(), Index_LastIndex));
         return propertyData(Index_LastIndex)->toInt32();
     }
     void setLastIndex(int index) {
-        Q_ASSERT(Index_LastIndex == internalClass()->find(engine()->id_lastIndex()->propertyKey()));
+        Q_ASSERT(internalClass()->verifyIndex(engine()->id_lastIndex()->propertyKey(), Index_LastIndex));
         if (!internalClass()->propertyData[Index_LastIndex].isWritable()) {
             engine()->throwTypeError();
             return;