Migrate to Rust 2024 edition#3515
Merged
stevenengler merged 12 commits intoshadow:mainfrom Feb 27, 2025
Merged
Conversation
sporksmith
approved these changes
Feb 24, 2025
Contributor
sporksmith
left a comment
There was a problem hiding this comment.
LGTM. I think the auto-formatting changes are an improvement and worth the churn.
This does not include uses of "gen" by other libraries (for example `rand::Rng::gen`).
This won't yet build. We'll fix the build issues in follow-up commits.
The default binding mode was changed to "ref" in rust 2021, and now in
rust 2024 it's an error to use the `ref` binding modifier when the
binding mode is already "ref".
For example:
```text
error: binding modifiers may only be written when the default binding mode is `move`
--> lib/tcp/src/buffer.rs:347:27
|
347 | Segment::Data(ref data) => data.len().try_into().unwrap(),
| ^^^ binding modifier not allowed under `ref` default binding mode
|
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/match-ergonomics.html>
```
```text
error[E0716]: temporary value dropped while borrowed
--> lib/shim/src/signals.rs:119:13
|
118 | let action = tls_process_shmem::with(|process| *unsafe {
| ________________________________________________________-
119 | | process.protected.borrow(&host_lock.root).signal_action(sig)
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement
| | |
| | creates a temporary value which is freed while still in use
120 | | });
| |_________- borrow later used here
```
This is unrelated to the rust 2024 migration, but I noticed this so decided to change it.
Lots of code churn here ¯\_(ツ)_/¯
946c0a1 to
b82416e
Compare
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
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.
We don't technically need to upgrade the edition, but I think we might as well get this out of the way so that we use the modern capture rules, scopes, etc. I didn't use
cargo fixfor any of this.There are some changes regarding capture rules, drop order, lifetime scopes, etc:
https://doc.rust-lang.org/edition-guide/rust-2024/language.html
A possible issue is that we have a lot of unsafe code and some may have relied on the old behaviour, which is now changed in rust 2025. But the updated code builds and passes all tests, so it's probably (hopefully) fine. I looked through the changes briefly, and I think it's unlikely that any broke our unsafe code.
Rustfmt now has "style editions". By upgrading rust editions, cargo automatically uses the new rustfmt style edition. This does have quite a bit of code churn. I think we could avoid this by specifying
style_edition = "2021"in arustfmt.tomlfile, but I decided to just do a rustfmt here using the latest (default) style edition. We can easily revert this before merging if anyone wants.Edit:
lint-cargo-lockrequires actions/runner-images#11637, but maybe it would be better to use the rust version we specify inci/rust-toolchain-stable.toml. The downside would be longer a workflow run time.