From 8c085c5722fb18eacf23f89ec9ba51a71d2510b2 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 28 Apr 2023 12:51:37 +0200 Subject: [PATCH] Fix test for move-only functor objects Amend 3c6e9dcc623c9d7281a81174bb3a696e030f30a7 by making sure that we explicitly move move-only functors into the slot object in the respective tests, and that failing to do so doesn't compile. Also add test coverage for mutable lambdas, which work as they do with connected functors: the connection stores a copy, and calls don't modify the original functor. Change-Id: I637e6f407133e2f8f72109b3fe5369a11d19da93 Reviewed-by: Thiago Macieira Reviewed-by: Qt CI Bot --- src/corelib/kernel/qobjectdefs_impl.h | 2 +- .../corelib/kernel/qobject/tst_qobject.cpp | 69 +++++++++++++++++-- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/corelib/kernel/qobjectdefs_impl.h b/src/corelib/kernel/qobjectdefs_impl.h index a950cf9d4e..8a9254a66d 100644 --- a/src/corelib/kernel/qobjectdefs_impl.h +++ b/src/corelib/kernel/qobjectdefs_impl.h @@ -503,7 +503,7 @@ namespace QtPrivate { if constexpr (QtPrivate::FunctionPointer::IsPointerToMemberFunction) { using ActualArguments = typename ActualSignature::Arguments; - return new QtPrivate::QSlotObject(func); + return new QtPrivate::QSlotObject(std::move(func)); } else { constexpr int MatchingArgumentCount = QtPrivate::countMatchingArguments(); using ActualArguments = typename QtPrivate::List_Left::Value; diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp index 82d46a3127..ede3bcccb2 100644 --- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp +++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp @@ -8363,6 +8363,11 @@ class AsyncCaller : public QObject { Q_OBJECT public: + ~AsyncCaller() + { + if (slotObject) + slotObject->destroyIfLastRef(); + } void callback0() {} void callback1(const QString &) {} void callbackInt(int) {} @@ -8377,9 +8382,12 @@ public: template bool callMe0(const typename QtPrivate::ContextTypeForFunctor::ContextType *, Functor &&func) { + if (slotObject) { + slotObject->destroyIfLastRef(); + slotObject = nullptr; + } QtPrivate::AssertCompatibleFunctions(); - auto *slotObject = QtPrivate::makeSlotObject(std::forward(func)); - slotObject->destroyIfLastRef(); + slotObject = QtPrivate::makeSlotObject(std::forward(func)); return true; } @@ -8392,9 +8400,12 @@ public: template bool callMe1(const typename QtPrivate::ContextTypeForFunctor::ContextType *, Functor &&func) { + if (slotObject) { + slotObject->destroyIfLastRef(); + slotObject = nullptr; + } QtPrivate::AssertCompatibleFunctions(); - auto *slotObject = QtPrivate::makeSlotObject(std::forward(func)); - slotObject->destroyIfLastRef(); + slotObject = QtPrivate::makeSlotObject(std::forward(func)); return true; } @@ -8403,10 +8414,13 @@ public: { return callMe1(nullptr, std::forward(func)); } + + QtPrivate::QSlotObjectBase *slotObject = nullptr; }; static void freeFunction0() {} static void freeFunction1(QString) {} +static void freeFunctionVariant(QVariant) {} template inline constexpr bool compiles(Functor &&) { @@ -8415,11 +8429,15 @@ inline constexpr bool compiles(Functor &&) { void tst_QObject::asyncCallbackHelper() { + QString arg1 = "Parameter"; + void *argv[] = { nullptr, &arg1 }; + auto lambda0 = []{}; auto lambda1 = [](const QString &) {}; auto lambda2 = [](const QString &, int) {}; const auto constLambda = [](const QString &) {}; auto moveOnlyLambda = [u = std::unique_ptr()]{}; + auto moveOnlyLambda1 = [u = std::unique_ptr()](const QString &){}; SlotFunctor functor0; SlotFunctorString functor1; @@ -8428,6 +8446,8 @@ void tst_QObject::asyncCallbackHelper() static_assert(compiles(&AsyncCaller::callback0)); static_assert(compiles(&AsyncCaller::staticCallback0)); static_assert(compiles(lambda0)); + static_assert(!compiles(moveOnlyLambda)); + static_assert(compiles(std::move(moveOnlyLambda))); static_assert(compiles(freeFunction0)); static_assert(compiles(functor0)); @@ -8442,6 +8462,8 @@ void tst_QObject::asyncCallbackHelper() static_assert(compiles(&AsyncCaller::callback1)); static_assert(compiles(&AsyncCaller::staticCallback1)); static_assert(compiles(lambda1)); + static_assert(!compiles(moveOnlyLambda1)); + static_assert(compiles(std::move(moveOnlyLambda1))); static_assert(compiles(constLambda)); static_assert(compiles(freeFunction1)); static_assert(compiles(functor1)); @@ -8470,6 +8492,8 @@ void tst_QObject::asyncCallbackHelper() // the return value through to anything. static_assert(compiles(&AsyncCaller::returnInt)); + static_assert(compiles(freeFunctionVariant)); + AsyncCaller caller; // with context QVERIFY(caller.callMe0(&caller, &AsyncCaller::callback0)); @@ -8477,7 +8501,7 @@ void tst_QObject::asyncCallbackHelper() QVERIFY(caller.callMe0(&caller, &AsyncCaller::staticCallback0)); QVERIFY(caller.callMe0(&caller, lambda0)); QVERIFY(caller.callMe0(&caller, freeFunction0)); -// QVERIFY(caller.callMe0(&caller, moveOnlyLambda)); + QVERIFY(caller.callMe0(&caller, std::move(moveOnlyLambda))); QVERIFY(caller.callMe1(&caller, &AsyncCaller::callback1)); QVERIFY(caller.callMe1(&caller, &AsyncCaller::staticCallback1)); @@ -8489,12 +8513,45 @@ void tst_QObject::asyncCallbackHelper() QVERIFY(caller.callMe0(&AsyncCaller::staticCallback0)); QVERIFY(caller.callMe0(lambda0)); QVERIFY(caller.callMe0(freeFunction0)); -// QVERIFY(caller.callMe0(moveOnlyLambda)); QVERIFY(caller.callMe1(&AsyncCaller::staticCallback1)); QVERIFY(caller.callMe1(lambda1)); QVERIFY(caller.callMe1(constLambda)); + QVERIFY(caller.callMe1(std::move(moveOnlyLambda1))); QVERIFY(caller.callMe1(freeFunction1)); + + { + struct MoveOnlyFunctor { + MoveOnlyFunctor() : payload("Hello World!") {} + MoveOnlyFunctor(MoveOnlyFunctor &&) = default; + MoveOnlyFunctor(const MoveOnlyFunctor &) = delete; + ~MoveOnlyFunctor() = default; + + void operator()() const { qDebug() << payload; } + QString payload; + } moveOnlyFunctor; + QVERIFY(caller.callMe0(std::move(moveOnlyFunctor))); + } + caller.slotObject->call(nullptr, argv); + + // mutable lambda; same behavior as mutableFunctor - we copy the functor + // in the QFunctorSlotObject, so the original is not modified + int status = 0; + auto mutableLambda1 = [&status, calls = 0]() mutable { status = ++calls; }; + + mutableLambda1(); + QCOMPARE(status, 1); + QVERIFY(caller.callMe0(mutableLambda1)); // this copies the lambda with count == 1 + caller.slotObject->call(nullptr, argv); // this doesn't change mutableLambda1, but the copy + QCOMPARE(status, 2); + mutableLambda1(); + QCOMPARE(status, 2); // and we are still at two + + auto mutableLambda2 = [calls = 0]() mutable { return ++calls; }; + QCOMPARE(mutableLambda2(), 1); + QVERIFY(caller.callMe0(mutableLambda2)); // this copies the lambda + caller.slotObject->call(nullptr, argv); // this call doesn't change mutableLambda2 + QCOMPARE(mutableLambda2(), 2); // so we are still at 2 } QTEST_MAIN(tst_QObject)