8000 Make .then() consume the task and defined moved-from task as invalid by danvratil · Pull Request #248 · qcoro/qcoro · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
< 8000 div id="start-of-content" class="show-on-focus">
/ qcoro Public
< 8000 /div>

Make .then() consume the task and defined moved-from task as invalid #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Make .then() consume the task and defined moved-from task as invalid #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions qcoro/impl/connect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ requires std::is_invocable_v<Callback> || std::is_invocable_v<Callback, T> || st
inline void connect(QCoro::Task<T> &&task, QObjectSubclass *context, Callback func) {
QPointer ctxWatcher = context;
if constexpr (std::is_same_v<T, void>) {
task.then([ctxWatcher, func = std::move(func)]() {
std::move(task).then([ctxWatcher, func = std::move(func)]() {
if (ctxWatcher) {
if constexpr (std::is_member_function_pointer_v<Callback>) {
(ctxWatcher->*func)();
Expand All @@ -30,7 +30,7 @@ inline void connect(QCoro::Task<T> &&task, QObjectSubclass *context, Callback fu
}
});
} else {
task.then([ctxWatcher, func = std::move(func)](auto &&value) {
std::move(task).then([ctxWatcher, func = std::move(func)](auto &&value) {
if (ctxWatcher) {
if constexpr (std::is_invocable_v<Callback, QObjectSubclass, T>) {
(ctxWatcher->*func)(std::forward<decltype(value)>(value));
Expand Down
8 changes: 7 additions & 1 deletion qcoro/impl/taskawaiterbase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,23 @@
#pragma once

#include "../qcorotask.h"
#include <QDebug>

namespace QCoro::detail
{

template<typename Promise>
inline bool TaskAwaiterBase<Promise>::await_ready() const noexcept {
return !mAwaitedCoroutine || mAwaitedCoroutine.done();
return mAwaitedCoroutine && mAwaitedCoroutine.done();
}

template<typename Promise>
inline void TaskAwaiterBase<Promise>::await_suspend(std::coroutine_handle<> awaitingCoroutine) noexcept {
if (!mAwaitedCoroutine) {
qWarning() << "QCoro::Task: Awaiting a default-constructed or a moved-from QCoro::Task<> - this will hang forever!";
return;
}

mAwaitedCoroutine.promise().addAwaitingCoroutine(awaitingCoroutine);
}

Expand Down
7 changes: 4 additions & 3 deletions qcoro/impl/taskbase.h
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,15 @@ inline auto TaskBase<T, TaskImpl, PromiseType>::operator co_await() const noexce

return TaskAwaiter{this->mCoroutine};
}

/*
template<typename T, template<typename> class TaskImpl, typename PromiseType>
template<typename ThenCallback>
requires (std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>))
inline auto TaskBase<T, TaskImpl, PromiseType>::then(ThenCallback &&callback) & {
// Provide a custom error handler that simply re-throws the current exception
return thenImplRef(*this, std::forward<ThenCallback>(callback), [](const auto &) { throw; });
}
*/

template<typename T, template<typename> class TaskImpl, typename PromiseType>
template<typename ThenCallback>
Expand All @@ -94,15 +95,15 @@ inline auto TaskBase<T, TaskImpl, PromiseType>::then(ThenCallback &&callback) &&
// would be destroyed before the thenImpl() is resumed and has a chance to consume/co_await the temporary LazyTask.
return thenImpl<TaskBase>(std::move(*this), std::forward<ThenCallback>(callback), [](const auto &) { throw; });
}

/*
template<typename T, template<typename> class TaskImpl, typename PromiseType>
template<typename ThenCallback, typename ErrorCallback>
requires ((std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>)) &&
std::is_invocable_v<ErrorCallback, const std::exception &>)
inline auto TaskBase<T, TaskImpl, PromiseType>::then(ThenCallback &&callback, ErrorCallback &&errorCallback) & {
return thenImplRef(*this, std::forward<ThenCallback>(callback), std::forward<ErrorCallback>(errorCallback));
}

*/
template<typename T, template<typename> class TaskImpl, typename PromiseType>
template<typename ThenCallback, typename ErrorCallback>
requires ((std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>)) &&
Expand Down
5 changes: 4 additions & 1 deletion qcoro/qcorotask.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,18 +388,21 @@ class TaskBase {
* result of the then() action can be co_awaited, if desired. If the callback
* returns an awaitable (Task<R>) then the result of then is the awaitable.
*/
/*
template<typename ThenCallback>
requires (std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>))
auto then(ThenCallback &&callback) &;
*/
template<typename ThenCallback>
requires (std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>))
auto then(ThenCallback &&callback) &&;


/*
template<typename ThenCallback, typename ErrorCallback>
requires ((std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>)) &&
std::is_invocable_v<ErrorCallback, const std::exception &>)
auto then(ThenCallback &&callback, ErrorCallback &&errorCallback) &;
*/
template<typename ThenCallback, typename ErrorCallback>
requires ((std::is_invocable_v<ThenCallback> || (!std::is_void_v<T> && std::is_invocable_v<ThenCallback, T>)) &&
std::is_invocable_v<ErrorCallback, const std::exception &>)
Expand Down
4 changes: 2 additions & 2 deletions qcoro/qml/qcoroqmltask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void QmlTask::then(QJSValue func) {
return;
}

d->task->then([func = std::move(func)](const QVariant &result) mutable -> void {
std::move(*d->task).then([func = std::move(func)](const QVariant &result) mutable -> void {
auto jsval = getEngineForValue(func)->toScriptValue(result);
func.call({jsval});
});
Expand All @@ -79,7 +79,7 @@ QmlTaskListener *QmlTask::await(const QVariant &intermediateValue)
if (!intermediateValue.isNull()) {
listener->setValue(QVariant(intermediateValue));
}
d->task->then([listener](auto &&value) {
std::move(*d->task).then([listener](auto &&value) {
if (listener) {
listener->setValue(std::move(value));
}
Expand Down
4 changes: 2 additions & 2 deletions qcoro/qml/qcoroqmltask.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct QCOROQML_EXPORT QmlTask {
*/
template <typename T>
QmlTask(QCoro::Task<T> &&task) : QmlTask(
task.then([](T &&result) -> QCoro::Task<QVariant> {
std::move(task).then([](T &&result) -> QCoro::Task<QVariant> {
co_return QVariant::fromValue(std::forward<T>(result));
}))
{
Expand All @@ -66,7 +66,7 @@ struct QCOROQML_EXPORT QmlTask {
*/
template <typename T = void>
QmlTask(QCoro::Task<> &&task) : QmlTask(
task.then([]() -> QCoro::Task<QVariant> {
std::move(task).then([]() -> QCoro::Task<QVariant> {
co_return QVariant();
}))
{
Expand Down
2 changes: 1 addition & 1 deletion qcoro/quick/qcoroimageprovider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ QQuickImageResponse *ImageProvider::requestImageResponse(const QString &id, cons

auto *response = new QCoroImageResponse();

task.then([response](QImage &&image) {
std::move(task).then([response](QImage &&image) {
response->reportFinished(std::move(image));
});

Expand Down
52 changes: 21 additions & 31 deletions tests/qcorotask.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,35 +395,6 @@ class QCoroTaskTest : public QCoro::TestObject<QCoroTaskTest>
QCOMPARE(result, QStringLiteral("42"));
}

QCoro::Task<> testMultipleAwaiters_coro(QCoro::TestContext) {
auto task = timer(100ms);

bool called = false;
// Internally co_awaits task
task.then([&called]() {
called = true;
});

co_await task;

QCORO_VERIFY(called);
}

QCoro::Task<> testMultipleAwaitersSync_coro(QCoro::TestContext ctx) {
ctx.setShouldNotSuspend();

auto task = []() -> QCoro::Task<> { co_return; }();

bool called = false;
task.then([&called]() {
called = true;
});

co_await task;

QCORO_VERIFY(called);
}

Q_SIGNAL void callbackCalled();

template <typename QObjectDerived, typename Signal>
Expand Down Expand Up @@ -462,6 +433,26 @@ class QCoroTaskTest : public QCoro::TestObject<QCoroTaskTest>
co_await verifySignalEmitted(this, &QCoroTaskTest::callbackCalled);
}

QCoro::Task<> testMovedFromTask_coro(QCoro::TestContext context) {
context.setTimeout(200ms);
context.expectTimeout();

const auto coro = []() -> QCoro::Task<int> {
co_await timer(1ms);
co_return 42;
};

auto task = coro();
auto task2 = std::move(task);

const int result = co_await task2;
QCORO_COMPARE(result, 42);

QTest::ignoreMessage(QtWarningMsg, "QCoro::Task: Awaiting a default-constructed or a moved-from QCoro::Task<> - this will hang forever!");
[[maybe_unused]] const int result2 = co_await task;
QCORO_FAIL("Awaiting a moved-from task should never resume the awaiter");
}

private Q_SLOTS:
addTest(SimpleCoroutine)
addTest(CoroutineValue)
Expand All @@ -484,8 +475,7 @@ private Q_SLOTS:
addTest(ThenErrorWithValue)
addTest(TaskConnect)
addThenTest(ImplicitArgumentConversion)
addTest(MultipleAwaiters)
addTest(MultipleAwaitersSync)
addTest(MovedFromTask)

// See https://github.com/danvratil/qcoro/issues/24
void testEarlyReturn()
Expand Down
11 changes: 11 additions & 0 deletions tests/testlibs/testobject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
// SPDX-License-Identifier: MIT

#include "testobject.h"
#include <chrono>

using namespace QCoro;

TestContext::TestContext(QEventLoop &el) : mEventLoop(&el) {
mEventLoop->setProperty("testFinished", false);
mEventLoop->setProperty("shouldNotSuspend", false);
mEventLoop->setProperty("expectTimeout", false);
}

TestContext::TestContext(TestContext &&other) noexcept {
Expand All @@ -30,3 +32,12 @@ TestContext &TestContext::operator=(TestContext &&other) noexcept {
void TestContext::setShouldNotSuspend() {
mEventLoop->setProperty("shouldNotSuspend", true);
}

void TestContext::setTimeout(std::chrono::milliseconds timeout) {
auto *timer = qobject_cast<QTimer *>(mEventLoop->property("timeout").value<QObject *>());
timer->start(timeout);
}

void TestContext::expectTimeout() {
mEventLoop->setProperty("expectTimeout", true);
}
34 changes: 28 additions & 6 deletions tests/testlibs/testobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class TestContext {
TestContext &operator=(const TestContext &) = delete;

void setShouldNotSuspend();
void setTimeout(std::chrono::milliseconds timeout);
void expectTimeout();

private:
QEventLoop *mEventLoop = {};
Expand Down Expand Up @@ -69,22 +71,42 @@ class TestObject : public QObject {

void coroWrapper(QCoro::Task<> (TestClass::*testFunction)(TestContext)) {
QEventLoop el;
QTimer::singleShot(5s, &el, [&el]() mutable { el.exit(1); });
QTimer timeout;
timeout.setSingleShot(true);
timeout.setInterval(5s);
timeout.callOnTimeout([&el]() mutable { el.exit(1); });
el.setProperty("timeout", QVariant::fromValue(&timeout));
timeout.start();

(static_cast<TestClass *>(this)->*testFunction)(el);

bool testFinished = el.property("testFinished").toBool();
const bool shouldNotSuspend = el.property("shouldNotSuspend").toBool();
const bool expectTimeout = el.property("expectTimeout").toBool();
if (testFinished) {
QVERIFY(shouldNotSuspend);
} else {
QVERIFY(!shouldNotSuspend);

const auto result = el.exec();
QVERIFY2(result == 0, "Test function has timed out");

testFinished = el.property("testFinished").toBool();
QVERIFY(testFinished);
const auto success = el.exec() == 0;
if (success) {
if (expectTimeout) {
QFAIL("Test function has not timed out as expected");
} else {
testFinished = el.property("testFinished").toBool();
QVERIFY(testFinished);
return;
}
} else {
if (expectTimeout) {
// Passed
return;
} else {
QFAIL("Test function has timed out");
}
}

Q_UNREACHABLE();
}
}
};
Expand Down
Loading
0