Skip to content

Add from_or_boxed constructor#17

Merged
eholk merged 3 commits intomicrosoft:mainfrom
eholk:from_or_box
Sep 6, 2022
Merged

Add from_or_boxed constructor#17
eholk merged 3 commits intomicrosoft:mainfrom
eholk:from_or_box

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
futures = { version = "0.3", features = ["executor"] }

[features]
alloc = []
Copy link
Contributor

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
Contributor

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