-
Notifications
You must be signed in to change notification settings - Fork 56
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.
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
Job handles, aka Here comes the future #754
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes for more idiomatic code, allows to invoke callbacks across threads, spares the library from clumsy function signatures, and prepares for connectSingleShot() deprecation.
KitsuneRal
force-pushed
the
kitsune/proxy-future
branch
3 times, most recently
from
May 13, 2024 08:02
d880c4c
to
6dcd7a8
Compare
Because this commit changes the returned type of Connection::callApi() to JobHandle<JobT>, it will break the code that initialises an `auto*` variable from Connection::callApi(). If the variable has a type of `SpecificJob*`, implicit conversion of QPointer should maintain compatibility; and if it's `auto` it can be later used as an ordinary QPointer, too. But now you get the QFuture interface at `job.`, while still having the job interface at `job->`.
[skip ci]
Both Apple Clang and MSVC fall through to the static_assert(false) that doesn't depend (in itself) on template variables and trip on it even though it is behind a constexpr if. MSVC also doesn't handle mutable keyword on a lambda without the parentheses and, separately, tumbles over with internal compiler error, no less, on User::doSetAvatar() - even its (almost unchanged) pre-future code upsets MSVC once callApi() starts returning JobHandle.
It should (ideally) come after all local dependencies are in. [skip ci]
With Qt::SingleShotConnection and QtFuture::connect() around, there's not much reason to have our means to the same end.
KitsuneRal
force-pushed
the
kitsune/proxy-future
branch
2 times, most recently
from
May 13, 2024 14:16
f9995d4
to
042442a
Compare
KitsuneRal
force-pushed
the
kitsune/proxy-future
branch
from
May 13, 2024 14:58
042442a
to
993624f
Compare
Skip is an empty structure, so we can optimise away pretty much the entire BoundFn structure when it's initialised with {Skip, Skip} (a case when then() is called with the single onSuccess argument). Unfortunately, Clang either crashes with ICE or emits faulty code when fn has no unique address (maybe because BoundFn::call() takes its address to determine if it's a function pointer? I don't know); so that no_unique_address is only added for compilers that can run with it.
KitsuneRal
force-pushed
the
kitsune/proxy-future
branch
from
May 13, 2024 18:07
993624f
to
4ab6e3b
Compare
TobiasFella
approved these changes
May 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The centrepiece of this is in 91df7a7, specifically in
jobhandle.cpp
- the rest is introducing eitherJobHandle
orQFuture
in various contexts, as a proof of concept.Closes #182.