From 9df44b2b502f9ab9a379c8454b000d2085aed744 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Sun, 23 Sep 2018 11:35:54 +0200 Subject: Refactor deletion of properties In line with the previous commit, allow entries with a valid PropertyKey, but invalid attributes in the InternalClass. Those entries mark a deleted property. This cleans up/unifies some of the code in the internal class implementation and allows re-using the slot if a deleted property gets added again. Change-Id: I1bada697486e3cafce7689bae87b7f884200dd99 Reviewed-by: Erik Verbruggen --- src/qml/jsruntime/qv4global_p.h | 1 + src/qml/jsruntime/qv4internalclass.cpp | 43 ++++++++++++++++------------------ src/qml/jsruntime/qv4internalclass_p.h | 31 +++++++++++++++--------- src/qml/jsruntime/qv4object.cpp | 11 ++++----- 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/src/qml/jsruntime/qv4global_p.h b/src/qml/jsruntime/qv4global_p.h index c53465d24e..eab519720f 100644 --- a/src/qml/jsruntime/qv4global_p.h +++ b/src/qml/jsruntime/qv4global_p.h @@ -364,6 +364,7 @@ struct PropertyAttributes bool isEmpty() const { return !m_all; } uint flags() const { return m_flags; } + uint all() const { return m_all; } bool operator==(PropertyAttributes other) { return m_all == other.m_all; diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp index 2916b09d76..ddb8542e07 100644 --- a/src/qml/jsruntime/qv4internalclass.cpp +++ b/src/qml/jsruntime/qv4internalclass.cpp @@ -309,10 +309,12 @@ static void addDummyEntry(InternalClass *newClass, PropertyHash::Entry e) Heap::InternalClass *InternalClass::changeMember(PropertyKey identifier, PropertyAttributes data, InternalClassEntry *entry) { - data.resolve(); + if (!data.isEmpty()) + data.resolve(); PropertyHash::Entry *e = findEntry(identifier); Q_ASSERT(e && e->index != UINT_MAX); uint idx = e->index; + Q_ASSERT(idx != UINT_MAX); if (entry) { entry->index = idx; @@ -323,7 +325,7 @@ Heap::InternalClass *InternalClass::changeMember(PropertyKey identifier, Propert if (data == propertyData.at(idx)) return static_cast(this); - Transition temp = { { identifier }, nullptr, (int)data.flags() }; + Transition temp = { { identifier }, nullptr, int(data.all()) }; Transition &t = lookupOrInsertTransition(temp); if (t.lookup) return t.lookup; @@ -413,7 +415,8 @@ Heap::InternalClass *InternalClass::nonExtensible() void InternalClass::addMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry) { Q_ASSERT(id.isStringOrSymbol()); - data.resolve(); + if (!data.isEmpty()) + data.resolve(); PropertyHash::Entry *e = object->internalClass()->findEntry(id); if (e) { changeMember(object, id, data, entry); @@ -427,7 +430,8 @@ void InternalClass::addMember(QV4::Object *object, PropertyKey id, PropertyAttri Heap::InternalClass *InternalClass::addMember(PropertyKey identifier, PropertyAttributes data, InternalClassEntry *entry) { Q_ASSERT(identifier.isStringOrSymbol()); - data.resolve(); + if (!data.isEmpty()) + data.resolve(); PropertyHash::Entry *e = findEntry(identifier); if (e) @@ -483,26 +487,17 @@ void InternalClass::removeChildEntry(InternalClass *child) void InternalClass::removeMember(QV4::Object *object, PropertyKey identifier) { +#ifndef QT_NO_DEBUG Heap::InternalClass *oldClass = object->internalClass(); - Q_ASSERT(oldClass->propertyTable.lookup(identifier) && oldClass->propertyTable.lookup(identifier)->index < oldClass->size); - - Transition temp = { { identifier }, nullptr, Transition::RemoveMember }; - Transition &t = object->internalClass()->lookupOrInsertTransition(temp); - - if (!t.lookup) { - // create a new class and add it to the tree - Heap::InternalClass *newClass = oldClass->engine->newClass(oldClass); - // simply make the entry inaccessible - int idx = newClass->propertyTable.removeIdentifier(identifier, oldClass->size); - newClass->nameMap.set(idx, PropertyKey::invalid()); - newClass->propertyData.set(idx, PropertyAttributes()); - t.lookup = newClass; - Q_ASSERT(t.lookup); - } - object->setInternalClass(t.lookup); + Q_ASSERT(oldClass->findEntry(identifier) != nullptr); +#endif + + changeMember(object, identifier, Attr_Invalid); +#ifndef QT_NO_DEBUG // we didn't remove the data slot, just made it inaccessible Q_ASSERT(object->internalClass()->size == oldClass->size); +#endif } Heap::InternalClass *InternalClass::sealed() @@ -610,10 +605,12 @@ Heap::InternalClass *InternalClass::propertiesFrozen() const frozen = frozen->changePrototype(prototype); for (uint i = 0; i < size; ++i) { PropertyAttributes attrs = propertyData.at(i); - if (attrs.isEmpty()) + if (!nameMap.at(i).isValid()) continue; - attrs.setWritable(false); - attrs.setConfigurable(false); + if (!attrs.isEmpty()) { + attrs.setWritable(false); + attrs.setConfigurable(false); + } frozen = frozen->addMember(nameMap.at(i), attrs); } return frozen->d(); diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h index 2d60a0048f..681cbda5f9 100644 --- a/src/qml/jsruntime/qv4internalclass_p.h +++ b/src/qml/jsruntime/qv4internalclass_p.h @@ -314,8 +314,7 @@ struct InternalClassTransition PrototypeChange = 0x201, ProtoClass = 0x202, Sealed = 0x203, - Frozen = 0x204, - RemoveMember = -1 + Frozen = 0x204 }; bool operator==(const InternalClassTransition &other) const @@ -378,7 +377,8 @@ struct InternalClass : Base { PropertyHash::Entry *e = propertyTable.lookup(id); if (e && e->index < size) { PropertyAttributes a = propertyData.at(e->index); - return { e->index, e->setterIndex, a }; + if (!a.isEmpty()) + return { e->index, e->setterIndex, a }; } return { UINT_MAX, UINT_MAX, Attr_Invalid }; @@ -395,8 +395,11 @@ struct InternalClass : Base { Q_ASSERT(id.isStringOrSymbol()); PropertyHash::Entry *e = propertyTable.lookup(id); - if (e && e->index < size) - return { e->index, propertyData.at(e->index) }; + if (e && e->index < size) { + PropertyAttributes a = propertyData.at(e->index); + if (!a.isEmpty()) + return { e->index, a }; + } return { UINT_MAX, Attr_Invalid }; } @@ -408,11 +411,13 @@ struct InternalClass : Base { PropertyHash::Entry *e = propertyTable.lookup(id); if (e && e->index < size) { PropertyAttributes a = propertyData.at(e->index); - if (a.isAccessor()) { - Q_ASSERT(e->setterIndex != UINT_MAX); - return { e->setterIndex, a }; + if (!a.isEmpty()) { + if (a.isAccessor()) { + Q_ASSERT(e->setterIndex != UINT_MAX); + return { e->setterIndex, a }; + } + return { e->index, a }; } - return { e->index, a }; } return { UINT_MAX, Attr_Invalid }; @@ -423,8 +428,10 @@ struct InternalClass : Base { Q_ASSERT(id.isStringOrSymbol()); PropertyHash::Entry *e = propertyTable.lookup(id); - if (e && e->index < size) + if (e && e->index < size) { + Q_ASSERT(!propertyData.at(e->index).isEmpty()); return e->index; + } return UINT_MAX; } @@ -434,8 +441,10 @@ struct InternalClass : Base { Q_ASSERT(id.isStringOrSymbol()); PropertyHash::Entry *e = propertyTable.lookup(id); - if (e && e->index < size) + if (e && e->index < size) { + Q_ASSERT(!propertyData.at(e->index).isEmpty()); return e->index == index; + } return false; } diff --git a/src/qml/jsruntime/qv4object.cpp b/src/qml/jsruntime/qv4object.cpp index b6446d39d1..ea374e43c0 100644 --- a/src/qml/jsruntime/qv4object.cpp +++ b/src/qml/jsruntime/qv4object.cpp @@ -364,18 +364,17 @@ PropertyKey ObjectOwnPropertyKeyIterator::next(const Object *o, Property *pd, Pr while (memberIndex < o->internalClass()->size) { PropertyKey n = o->internalClass()->nameMap.at(memberIndex); ++memberIndex; - if (!n.isStringOrSymbol()) { + if (!n.isStringOrSymbol()) // accessor properties have a dummy entry with n == 0 continue; - } - if (!iterateOverSymbols && n.isSymbol()) { + if (!iterateOverSymbols && n.isSymbol()) continue; - } - if (iterateOverSymbols && !n.isSymbol()) { + if (iterateOverSymbols && !n.isSymbol()) continue; - } InternalClassEntry e = o->internalClass()->find(n); + if (!e.isValid()) + continue; if (pd) { pd->value = *o->propertyData(e.index); if (e.attributes.isAccessor()) -- cgit v1.2.3