Skip to content

Make StackFuture::from fail statically if the future doesn't fit#20

Merged
eholk merged 4 commits intomicrosoft:mainfrom
eholk:static-size-check
Nov 10, 2022
Merged

Make StackFuture::from fail statically if the future doesn't fit#20
eholk merged 4 commits intomicrosoft:mainfrom
eholk:static-size-check

Conversation

@eholk
Copy link
Copy Markdown
Contributor

@eholk eholk commented Nov 9, 2022

Thanks to @jstarks for the suggestion of how to do this!

This also bumps the version in Cargo.toml to 0.3.0 since it's a breaking change now that StackFuture::from will fail to compile in cases where it would have panicked at runtime.

@eholk eholk requested review from daprilik and jstarks November 9, 2022 00:21
@eholk eholk mentioned this pull request Nov 9, 2022
@eholk
Copy link
Copy Markdown
Contributor Author

eholk commented Nov 9, 2022

Weird that the static check isn't working on miri. I wonder if this is a bug around const evaluation with miri?

@eholk eholk force-pushed the static-size-check branch from 79eb825 to 9cec300 Compare November 10, 2022 00:26
Copy link
Copy Markdown
Contributor

@daprilik daprilik left a comment

Choose a reason for hiding this comment

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

LGTM!

@eholk eholk merged commit 9a27962 into microsoft:main Nov 10, 2022
@eholk eholk deleted the static-size-check branch November 10, 2022 00:36
@eholk
Copy link
Copy Markdown
Contributor Author

eholk commented Nov 10, 2022

Weird that the static check isn't working on miri. I wonder if this is a bug around const evaluation with miri?

I asked about this on Zulip. It seems like what's probably going on is that the compilation failure happens post-monomorphization while Miri runs pre-monomorphization. If we remove the compile_fail, the test actually fails under both configurations, so it's not that this check escapes Miri, it's just that rustdoc interprets the failure differently.

That makes me feel okay about adding a Miri-specific workaround for those tests.

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