Prepare for stabilization of sanitizers #376
Conversation
|
Let me know when this is ready for review. |
|
It is ready for review now! The only remaining item is to fill in the actual version when the sanitizers are stabilized, which is not decided by the Rust project yet. |
|
Cool, thanks. I'll take a look on Monday. |
src/project.rs
Outdated
| let rust_version_string = rust_version_string()?; | ||
| let rust_version = | ||
| RustVersion::from_str(&rust_version_string).map_err(|e| anyhow::anyhow!(e))?; |
There was a problem hiding this comment.
Nitpick: seems like these two steps could be collapsed into a single, fallible RustVersion constructor:
impl RustVersion {
pub fn discover() -> anyhow::Result<Self> {
let version_string = rust_version_string()?;
let me = Self::from_str(&version_string)?;
Ok(me)
}
}No need to make API-consumers do two things when they could do one; just an opportunity for them to mess things up in between or not connect the two interfaces properly.
There was a problem hiding this comment.
The reason these are two separate steps is that we need the version string too, to detect whether the compiler is nightly or not.
I considered adding the version string to the RustVersion struct, but that would make it no longer Copy, and neither API looked obviously better than the other.
If you want me to encapsulate everything, I guess I could turn RustVersion into this:
pub struct RustVersion {
pub major: u32,
pub minor: u32,
// we don't care about the patch version and it's a bit of a pain to parse
pub nightly: bool,
}and with the nightly flag being resolved in RustVersion we don't need to expose the string outside the module at all.
|
@fitzgen Thanks for the review! All good points, thank you! I think I've addressed everything, so I'd appreciate you taking another look. |
The stabilization of sanitizers is expected to remove the
-Zsanitizerflag we rely on: rust-lang/rust#123617This PR adds the logic to handle the compiler options required on newer rustc versions, both stable and nightly.
TODO: wait until sanitizers are actually stabilized, and fill in the correct rustc version on which they were stabilized. Hence the draft status - the code is complete otherwise.