From ff04810bd09d3897ccede880680dd94fcf585171 Mon Sep 17 00:00:00 2001 From: Brett Stottlemyer Date: Mon, 20 May 2019 14:08:32 -0400 Subject: Make sure class enums work dynamically This includes making sure typed class enums get the right size. This also fixes other enums that were incorrectly tested as variants. Change-Id: Ie330ceee91fe4192a11405a4e231f1750247cd5e Reviewed-by: Michael Brasser --- src/remoteobjects/qremoteobjectdynamicreplica.cpp | 19 +++++++++--- src/remoteobjects/qremoteobjectnode.cpp | 37 +++++++++++++++++++---- src/remoteobjects/qremoteobjectpacket.cpp | 25 +++++++++++---- tests/auto/proxy_multiprocess/client/main.cpp | 21 +++++++------ tests/auto/proxy_multiprocess/namespace.h | 6 ++++ tests/auto/proxy_multiprocess/server/main.cpp | 2 ++ tests/auto/proxy_multiprocess/subclass.rep | 2 ++ 7 files changed, 87 insertions(+), 25 deletions(-) diff --git a/src/remoteobjects/qremoteobjectdynamicreplica.cpp b/src/remoteobjects/qremoteobjectdynamicreplica.cpp index f778c3a..a004593 100644 --- a/src/remoteobjects/qremoteobjectdynamicreplica.cpp +++ b/src/remoteobjects/qremoteobjectdynamicreplica.cpp @@ -184,10 +184,21 @@ int QRemoteObjectDynamicReplica::qt_metacall(QMetaObject::Call call, int id, voi QVariantList args; args.reserve(typeSize); for (int i = 0; i < typeSize; ++i) { - if (impl->m_metaObject->indexOfEnumerator(types[i].constData()) != -1) - args.push_back(QVariant(QMetaType::Int, argv[i + 1])); - else - args.push_back(QVariant(QMetaType::type(types[i].constData()), argv[i + 1])); + const int type = QMetaType::type(types[i].constData()); + if (impl->m_metaObject->indexOfEnumerator(types[i].constData()) != -1) { + const auto size = QMetaType(type).sizeOf(); + switch (size) { + case 1: args.push_back(QVariant(QMetaType::Char, argv[i + 1])); break; + case 2: args.push_back(QVariant(QMetaType::Short, argv[i + 1])); break; + case 4: args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break; + // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int) +// case 8: args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break; + default: + qWarning() << "Invalid enum detected (Dynamic Replica)" << QMetaType::typeName(type) << "with size" << size; + args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break; + } + } else + args.push_back(QVariant(type, argv[i + 1])); } if (debugArgs) { diff --git a/src/remoteobjects/qremoteobjectnode.cpp b/src/remoteobjects/qremoteobjectnode.cpp index 3353de1..127ed9d 100644 --- a/src/remoteobjects/qremoteobjectnode.cpp +++ b/src/remoteobjects/qremoteobjectnode.cpp @@ -703,7 +703,7 @@ struct EnumPair { struct EnumData { QByteArray name; bool isFlag, isScoped; - quint32 keyCount; + quint32 keyCount, size; QVector values; }; @@ -727,9 +727,30 @@ static void registerEnum(const QByteArray &name, const QMetaObject *meta, int si if (QMetaType::isRegistered(QMetaType::type(name))) return; static const auto flags = QMetaType::IsEnumeration | QMetaType::NeedsConstruction | QMetaType::NeedsDestruction; - int id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, - &EnumConstructor, size, flags, meta); - qCDebug(QT_REMOTEOBJECT) << "Registering new enum with id" << id << name; + int id; + switch (size) { + case 1: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, + &EnumConstructor, size, flags, meta); + break; + case 2: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, + &EnumConstructor, size, flags, meta); + break; + case 4: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, + &EnumConstructor, size, flags, meta); + break; + // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int) +// case 8: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, +// &EnumConstructor, size, flags, meta); +// break; + default: + qWarning() << "Invalid enum detected" << name << "with size" << size << ". Defaulting to register as int."; + id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor, + &EnumConstructor, size, flags, meta); + } +#ifdef QTRO_VERBOSE_PROTOCOL + qDebug() << "Registering new enum with id" << id << name << "size:" << size; +#endif + qCDebug(QT_REMOTEOBJECT) << "Registering new enum with id" << id << name << "size:" << size; } static int registerGadgets(IoDeviceBase *connection, Gadgets &gadgets, QByteArray typeName) @@ -768,7 +789,7 @@ static int registerGadgets(IoDeviceBase *connection, Gadgets &gadgets, QByteArra const auto enumCount = meta->enumeratorCount(); for (int i = 0; i < enumCount; i++) { const QByteArray registeredName = QByteArray(typeName).append("::").append(meta->enumerator(i).name()); - registerEnum(registeredName, meta); + registerEnum(registeredName, meta, gadget.enums.at(i).size); } QMetaType::TypeFlags flags = QMetaType::IsGadget; int gadgetTypeId; @@ -806,6 +827,7 @@ static void deserializeEnum(QDataStream &ds, EnumData &enumData) ds >> enumData.name; ds >> enumData.isFlag; ds >> enumData.isScoped; + ds >> enumData.size; ds >> enumData.keyCount; for (quint32 i = 0; i < enumData.keyCount; i++) { EnumPair pair; @@ -871,12 +893,14 @@ QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connec builder.setClassName(type); in >> numEnums; + QVector enumSizes(numEnums); for (quint32 i = 0; i < numEnums; ++i) { EnumData enumData; deserializeEnum(in, enumData); auto enumBuilder = builder.addEnumerator(enumData.name); enumBuilder.setIsFlag(enumData.isFlag); enumBuilder.setIsScoped(enumData.isScoped); + enumSizes[i] = enumData.size; for (quint32 k = 0; k < enumData.keyCount; ++k) { const auto pair = enumData.values.at(k); @@ -935,10 +959,11 @@ QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connec // We only want to register the new enumerations, and since we just added them, we know they // are the last indices. Thus a backwards count seems most efficient. const int totalEnumCount = meta->enumeratorCount(); + int incrementingIndex = 0; for (int i = numEnums; i > 0; i--) { auto const enumMeta = meta->enumerator(totalEnumCount - i); const QByteArray registeredName = QByteArray(type).append("::").append(enumMeta.name()); - registerEnum(registeredName, meta); + registerEnum(registeredName, meta, enumSizes.at(incrementingIndex++)); } dynamicTypes.insert(typeString, meta); return meta; diff --git a/src/remoteobjects/qremoteobjectpacket.cpp b/src/remoteobjects/qremoteobjectpacket.cpp index 631c362..4b8f77c 100644 --- a/src/remoteobjects/qremoteobjectpacket.cpp +++ b/src/remoteobjects/qremoteobjectpacket.cpp @@ -81,11 +81,21 @@ namespace QRemoteObjectPackets { const QVariant encodeVariant(const QVariant &value) { if (QMetaType::typeFlags(value.userType()).testFlag(QMetaType::IsEnumeration)) { + auto converted = QVariant(value); + const auto size = QMetaType(value.userType()).sizeOf(); + switch (size) { + case 1: converted.convert(QMetaType::Char); break; + case 2: converted.convert(QMetaType::Short); break; + case 4: converted.convert(QMetaType::Int); break; + // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int) +// case 8: converted.convert(QMetaType::Long); break; // typeId for long from qmetatype.h + default: + qWarning() << "Invalid enum detected" << QMetaType::typeName(value.userType()) << "with size" << size; + converted.convert(QMetaType::Int); + } #ifdef QTRO_VERBOSE_PROTOCOL - qDebug() << "Converting from enum to integer type" << value << value.value(); + qDebug() << "Converting from enum to integer type" << size << converted << value; #endif - auto converted = QVariant(value); - converted.convert(2); // typeId for int from qmetatype.h return converted; } return value; @@ -95,11 +105,11 @@ QVariant &decodeVariant(QVariant &value, int type) { if (QMetaType::typeFlags(type).testFlag(QMetaType::IsEnumeration)) { #ifdef QTRO_VERBOSE_PROTOCOL - int asInt = value.value(); + QVariant encoded(value); #endif value.convert(type); #ifdef QTRO_VERBOSE_PROTOCOL - qDebug() << "Converting to enum from integer type" << value << asInt; + qDebug() << "Converting to enum from integer type" << value << encoded; #endif } return value; @@ -363,8 +373,11 @@ static void serializeEnum(QDataStream &ds, const QMetaEnum &enumerator) ds << QByteArray::fromRawData(enumerator.name(), qstrlen(enumerator.name())); ds << enumerator.isFlag(); ds << enumerator.isScoped(); + const auto typeName = QByteArray(enumerator.scope()).append("::").append(enumerator.name()); + quint32 size = QMetaType(QMetaType::type(typeName.constData())).sizeOf(); + ds << size; #ifdef QTRO_VERBOSE_PROTOCOL - qDebug(" Enum (name = %s, isFlag = %s, isScoped = %s):", enumerator.name(), enumerator.isFlag() ? "true" : "false", enumerator.isScoped() ? "true" : "false"); + qDebug(" Enum (name = %s, size = %d, isFlag = %s, isScoped = %s):", enumerator.name(), size, enumerator.isFlag() ? "true" : "false", enumerator.isScoped() ? "true" : "false"); #endif const int keyCount = enumerator.keyCount(); ds << keyCount; diff --git a/tests/auto/proxy_multiprocess/client/main.cpp b/tests/auto/proxy_multiprocess/client/main.cpp index 5f11402..19ba46f 100644 --- a/tests/auto/proxy_multiprocess/client/main.cpp +++ b/tests/auto/proxy_multiprocess/client/main.cpp @@ -60,16 +60,18 @@ private Q_SLOTS: QCOMPARE(m_rep->subClass()->i(), initialI); QVERIFY(m_rep->tracks() != nullptr); QVERIFY(tracksSpy.count() || tracksSpy.wait()); - QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::bar)); - QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::SystemLocaleShortDate)); - QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Bravo)); + QCOMPARE(m_rep->myEnum(), ParentClassReplica::bar); + QCOMPARE(m_rep->date(), Qt::SystemLocaleShortDate); + QCOMPARE(m_rep->nsEnum(), NS::Bravo); + QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Bravo); QCOMPARE(m_rep->variant(), QVariant::fromValue(42.0f)); } else { QVERIFY(m_rep->subClass() == nullptr); QVERIFY(m_rep->tracks() == nullptr); - QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::foo)); - QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::ISODate)); - QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Alpha)); + QCOMPARE(m_rep->myEnum(), ParentClassReplica::foo); + QCOMPARE(m_rep->date(), Qt::ISODate); + QCOMPARE(m_rep->nsEnum(), NS::Alpha); + QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Alpha); QCOMPARE(m_rep->variant(), QVariant()); } @@ -97,9 +99,10 @@ private Q_SLOTS: QCOMPARE(m_rep->subClass()->myPOD(), updatedValue); QCOMPARE(m_rep->subClass()->i(), updatedI); QVERIFY(m_rep->tracks() != nullptr); - QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::foobar)); - QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::SystemLocaleLongDate)); - QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Charlie)); + QCOMPARE(m_rep->myEnum(), ParentClassReplica::foobar); + QCOMPARE(m_rep->date(), Qt::SystemLocaleLongDate); + QCOMPARE(m_rep->nsEnum(), NS::Charlie); + QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Charlie); QCOMPARE(m_rep->variant(), QVariant::fromValue(podValue)); qDebug() << "Verified expected final states, cleaning up."; } diff --git a/tests/auto/proxy_multiprocess/namespace.h b/tests/auto/proxy_multiprocess/namespace.h index 0b370ba..2d06e67 100644 --- a/tests/auto/proxy_multiprocess/namespace.h +++ b/tests/auto/proxy_multiprocess/namespace.h @@ -7,3 +7,9 @@ namespace NS Q_ENUM_NS(NamespaceEnum) } +namespace NS2 +{ + Q_NAMESPACE + enum class NamespaceEnum : quint8 { Alpha=1, Bravo, Charlie }; + Q_ENUM_NS(NamespaceEnum) +} diff --git a/tests/auto/proxy_multiprocess/server/main.cpp b/tests/auto/proxy_multiprocess/server/main.cpp index 9e806b9..1cd005d 100644 --- a/tests/auto/proxy_multiprocess/server/main.cpp +++ b/tests/auto/proxy_multiprocess/server/main.cpp @@ -58,6 +58,7 @@ private Q_SLOTS: parent.setMyEnum(ParentClassSource::bar); parent.setDate(Qt::SystemLocaleShortDate); parent.setNsEnum(NS::Bravo); + parent.setNs2Enum(NS2::NamespaceEnum::Bravo); parent.setVariant(QVariant::fromValue(42.0f)); } @@ -86,6 +87,7 @@ private Q_SLOTS: parent.setMyEnum(ParentClassSource::foobar); parent.setDate(Qt::SystemLocaleLongDate); parent.setNsEnum(NS::Charlie); + parent.setNs2Enum(NS2::NamespaceEnum::Charlie); parent.setVariant(QVariant::fromValue(podValue)); emit parent.enum2(ParentClassSource::foo, ParentClassSource::bar); diff --git a/tests/auto/proxy_multiprocess/subclass.rep b/tests/auto/proxy_multiprocess/subclass.rep index b1fe352..2538b88 100644 --- a/tests/auto/proxy_multiprocess/subclass.rep +++ b/tests/auto/proxy_multiprocess/subclass.rep @@ -3,6 +3,7 @@ USE_ENUM(Qt::DateFormat) USE_ENUM(NS::NamespaceEnum) +USE_ENUM(NS2::NamespaceEnum) POD MyPOD(int i, float f, QString s) POD VariantPOD(int i, int j) @@ -21,6 +22,7 @@ class ParentClass PROP(QVariant variant) PROP(Qt::DateFormat date = Qt::ISODate) PROP(NS::NamespaceEnum nsEnum = NS::Alpha) + PROP(NS2::NamespaceEnum ns2Enum = NS2::NamespaceEnum::Alpha) SLOT(bool start()) SLOT(bool quit()) -- cgit v1.2.3