Skip to content
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

Add from_or_boxed constructor #17

Merged
merged 3 commits into from
Sep 6, 2022
Merged

Add from_or_boxed constructor #17

merged 3 commits into from
Sep 6, 2022

Conversation

eholk
Copy link
Contributor

@eholk eholk commented Sep 3, 2022

This gives us a way to support any size future with a small value optimization. The from_or_boxed constructor attempts to store the wrapped future in StackFuture's reserved space, but if it cannot then it store the future in a box and wrap the box instead.

Note that doing this requires adding an "alloc" feature so this crate can continue to be used in pure no_std environments.

This also changes the panic behavior of try_from to not panic on alignment errors. To make it easier to determine why something went wrong, we make the has_*_for* functions public. We also adjust the panic messages in StackFuture::from to make it clear which conditions failed.

Finally, we introduce a testing matrix for GitHub actions to make sure we run tests both with and without allocation.

This gives us a way to support any size future with a small value
optimization. The `from_or_boxed` constructor attempts to store the
wrapped future in `StackFuture`'s reserved space, but if it cannot then
it store the future in a box and wrap the box instead.

Note that doing this requires adding an "alloc" feature so this crate
can continue to be used in pure no_std environments.

This also changes the panic behavior of `try_from` to not panic on
alignment errors. To make it easier to determine why something went
wrong, we make the `has_*_for*` functions public. We also adjust the
panic messages in `StackFuture::from` to make it clear which conditions
failed.

Finally, we introduce a testing matrix for GitHub actions to make sure
we run tests both with and without allocation.
@eholk eholk requested a review from daprilik September 3, 2022 00:32
@@ -16,3 +16,6 @@ categories = ["asynchronous", "no-std", "rust-patterns"]

[dev-dependencies]
futures = { version = "0.3", features = ["executor"] }

[features]
alloc = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you, but I'd also add default = ["alloc"], since I'd wager most folks are going to be running this with an allocator.

Note that you'll need to change some of the CI stuff as well

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and this is most certainly a semver breaking change, so it'll need to coincide with a version bump to 0.2

Introducing the alloc dependency by default is definitely semver-breaking
@eholk eholk merged commit 3b9876b into microsoft:main Sep 6, 2022
@eholk eholk deleted the from_or_box branch September 6, 2022 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants