Closed
Conversation
Release 0.2.0
* Fixed multiple things related to ownership and the usage of PIMPL: First, classes capturing the this-pointer should be non-copyable to prevent use-after-free bugs. Second, after they are non-copyable, users can't store them in the node-class, so a PIMPL-wrapper became necessary for the service client. Third, PIMPL-wrappers should generally mirror the impl-API, because there is no reason for arbitrary deviations. For this reason I added the promise-based functions into the impl-classes and the callback-based functions into the actual non-impl classes. Fourth, I decided on ownership semantics of the service client: No bookkeeping is provided for it since having a client without a handle to it is useless. PIMPL however is still useful to avoid users having to use shared pointers everywhere. * Enabling nested coroutines (#9) This fixes the promises and allows for nested coroutines. Also fixes the coroutine frame memory leaks.
…ince we use the result in the public API, this implies the user would have to handle the none state as well, but we actually guarantee that there is always either a value or error, never none. So this guarantee should be reflected in the type. On the other hand, I actually need the three states for the promise state, because a promise might not have anything yet. So I actually need a two-state type (a real result type, like std::expected and rust's Result) for the public API as well as a three-state PromiseState type for the promise.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
I've fixed the issue that the result type had three states: value, error or none. Since the result type is used in the public API, users would have to handle the 'none' state as well. However, we guarantee that there will always be either a 'value' or an 'error', never 'none'. This guarantee should therefore also be reflected in the type. On the other hand, I actually need the three states for the promise state, because a promise might not have a value or error yet. Therefore, I actually need both a two-state type (a real result type, like C++23's std::expected/Rust's Result) for the public API, as well as a three-state type
PromiseStatefor the promise.