Skip to content

RFC: Use stable Rust#512

Closed
bradjc wants to merge 2 commits intomasterfrom
use-stable-rust
Closed

RFC: Use stable Rust#512
bradjc wants to merge 2 commits intomasterfrom
use-stable-rust

Conversation

@bradjc
Copy link
Copy Markdown
Contributor

@bradjc bradjc commented Aug 23, 2023

Opening this on top of #511 as an idea to switch to using stable rust. Since we don't have to chase specific nightly versions, it seems like a good idea to just use stable rust with a minimum version for specific features we want.

But, this toolchain selection seems very confusing because again rust seems like it doesn't really support cross-compiling. What we want are very separate ways to specify the toolchain for building for embedded targets from ways to specify the toolchain for running host-side tests. It remains perplexing to me that rust still doesn't really seem to take this seriously.

Specifically, I don't know how to tell cargo "use nightly, but use a nightly with miri available". Maybe it does this automatically since we are running a miri command? I don't know.

Johnathan Van Why and others added 2 commits August 22, 2023 16:30
The semantics of int-to-pointer casts under strict provenance has changed. Ideally, we would update our `From<usize> for Register` implementation to use the `sptr` crate, but `sptr` does not currently have a suitable license. There is a [PR](Gankra/sptr#14) to change that, but it hasn't gotten a response in 7 months, so I don't think we can use it. Instead, this copies the implementation of `core::ptr::invalid`.

This also bumps our `thiserror` dependency, which forces Cargo to pick a newer version of `proc-macro2` that is compatible with recently nightly toolchains.
Only use nightly for miri.
Comment thread Makefile
--target=riscv32imac-unknown-none-elf --workspace
MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check" \
cargo miri test $(EXCLUDE_MIRI) --workspace
cargo +nightly miri test $(EXCLUDE_MIRI) --workspace
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We need to pin to a particular nightly release, otherwise this will break randomly as new nightly toolchains come out.

@jrvanwhy
Copy link
Copy Markdown
Collaborator

Specifically, I don't know how to tell cargo "use nightly, but use a nightly with miri available". Maybe it does this automatically since we are running a miri command? I don't know.

It won't. rustup determines the toolchain version, cargo parses the miri command. They're not the same tool, and by the time anything knows you need Miri you're already running the wrong toolchain version.

@jrvanwhy
Copy link
Copy Markdown
Collaborator

What we want are very separate ways to specify the toolchain for building for embedded targets from ways to specify the toolchain for running host-side tests. It remains perplexing to me that rust still doesn't really seem to take this seriously.

I did some digging and found the reasoning here: rust-lang/rustup#3355. It seems the underlying issue is that because user-defined toolchain names exist, rustup's argument syntax is limiting. It may still be possible, but I don't think it's worth the effort for us to pursue it.

@bradjc
Copy link
Copy Markdown
Contributor Author

bradjc commented Aug 23, 2023

I did some digging and found the reasoning here: rust-lang/rustup#3355. It seems the underlying issue is that because user-defined toolchain names exist, rustup's argument syntax is limiting. It may still be possible, but I don't think it's worth the effort for us to pursue it.

I'm not arguing with you, but these people built an entire programming language, they could figure out how to handle host-side testing needing a different compiler than a binary for an embedded system (if they wanted to).

@bradjc
Copy link
Copy Markdown
Contributor Author

bradjc commented Aug 24, 2023

Improved by #513

@bradjc bradjc closed this Aug 24, 2023
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