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

assert platform-minimum requirements at build time #3797

Merged
merged 4 commits into from Oct 19, 2021

Conversation

udoprog
Copy link
Contributor

@udoprog udoprog commented May 17, 2021

Adds assertions in build.rs which ensures that the platform being built for supports the minimum required by Tokio.

Motivation

Tokio currently makes a number of soft assumptions about the size of a usize (and isize), usually through the form of casting a u32 like:

let value: u32 = todo!();
value as usize

Arguably the preferable way to do this would be to perform a checked conversion using TryFrom, but it would be a lot of work to trace all of them down and it might not always be immediately feasible when it comes to things like bit flags.

It is unlikely that someone will ever attempt to build Tokio for a platform where usize is not at least 4 bytes, but at least when they do, they should be firmly shown that it's a bad idea until assumptions around usize et. al. have been fixed.

Solution

Add assertions to build.rs. I started tinkering with constant assertions, but Rust 1.45 is a bit too limiting to make them ergonomical. While it's possible that build.rs is run with a different rustc than the one used to compile the project, one would have to do something strange to have it do so.

@udoprog udoprog added C-maintenance Category: PRs that clean code up or issues documenting cleanup. A-tokio Area: The main tokio crate labels May 17, 2021
tokio/build.rs Outdated
Comment on lines 32 to 34
if mem::size_of::<usize>() < 4 {
panic!("Tokio only works correctly if usize is at least 4 bytes.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What about cross-compilation? Then the build script is not built in the target architecture. Another option is this:

#[cfg_attr(target_pointer_width = "16", compile_error("Pointer width must be at least 32"))]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. While strictly speaking not exhaustive, that's probably good enough until an MSRV bump. If someone designs a 31-bit hobby platform today they only have themselves to blame. :D

Copy link
Contributor Author

@udoprog udoprog May 17, 2021

Choose a reason for hiding this comment

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

How about this until we have sufficient const eval?

#[cfg(not(any(target_pointer_width = "32", target_pointer_width = "64", target_pointer_width = "128")))]
compile_error! {
    "Tokio requires the platform pointer width to be 32, 64, or 128 bits"
}

@Darksonn Darksonn merged commit 44e9013 into tokio-rs:master Oct 19, 2021
oliver-giersch pushed a commit to oliver-giersch/tokio that referenced this pull request Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-maintenance Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants