From 7d5ce89dadb8058f9c7144d890c33ce5aacb9c0d Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 19 Aug 2024 15:13:23 +0200 Subject: [PATCH 1/2] Fix type ambiguity in IO tests --- io/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/io/src/lib.rs b/io/src/lib.rs index 1fc5dd53e..abf43727f 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -349,8 +349,7 @@ mod tests { // checks we can attempt to read from a now-empty reader. let fill = BufRead::fill_buf(&mut slice).unwrap(); - assert_eq!(fill.len(), 0); - assert_eq!(fill, &[]); + assert!(fill.is_empty()); } #[test] From ad34a98c61d45126c109635457b31d91ed915030 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sun, 18 Aug 2024 13:36:10 +0200 Subject: [PATCH 2/2] Refactor Rust version checking Conditional compilation depending on Rust version using `cfg` had the disadvantage that we had to write the same code multiple times, compile it multiple times, execute it multiple times, update it multiple times... Apart from obvious maintenance issues the build script wasn't generating the list of allowed `cfg`s so those had to be maintained manually in `Cargo.toml`. This was fixable by printing an appropriate line but it's best to do it together with the other changes. Because we cannot export `cfg` flags from a crate to different crates we take a completely different approach: we define a macro called `rust_version` that takes a very naturally looking condition such as `if >= 1.70 {}`. This macro is auto-generated so that it produces different results based on the compiler version - it either expands to first block or the second block (after `else`). This way, the other crates can simply call the macro when needed. Unfortunately some minimal maintenance is still needed: to update the max version number when a newer version is used. (Note that code will still work with higher versions, it only limits which conditions can be used in downstream code.) This can be automated with the pin update script or we could just put the pin file into the `internals` directory and read the value from there. Not automating isn't terrible either since anyone adding a cfg with higher version will see a nice error about unknown version of Rust and can update it manually. Because this changes syntax to a more naturally looking version number, as a side effect the `cond_const` macro could be also updated to use the new macro under the hood, providing much nicer experience - it is no longer needed to provide human-readable version of the version string to put in the note about `const`ness requiring a newer version. As such the note is now always there using a single source of truth. It's also a great moment to introduce this change right now since there's currently no conditional compilation used in `bitcoin` crate making the changes minimal. --- Cargo-minimal.lock | 3 + Cargo-recent.lock | 3 + bitcoin/build.rs | 37 ---------- internals/Cargo.toml | 2 +- internals/build.rs | 49 ++++++++++++-- internals/src/array_vec.rs | 2 +- internals/src/const_tools.rs | 50 +++++++------- internals/src/lib.rs | 7 ++ io/Cargo.toml | 5 +- io/build.rs | 37 ---------- io/src/bridge.rs | 128 ++++++++++++++++++----------------- 11 files changed, 153 insertions(+), 170 deletions(-) delete mode 100644 bitcoin/build.rs delete mode 100644 io/build.rs diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 1b19231fe..16c7e5987 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -95,6 +95,9 @@ dependencies = [ [[package]] name = "bitcoin-io" version = "0.1.2" +dependencies = [ + "bitcoin-internals", +] [[package]] name = "bitcoin-primitives" diff --git a/Cargo-recent.lock b/Cargo-recent.lock index dc727fe4b..fb2340a80 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -94,6 +94,9 @@ dependencies = [ [[package]] name = "bitcoin-io" version = "0.1.2" +dependencies = [ + "bitcoin-internals", +] [[package]] name = "bitcoin-primitives" diff --git a/bitcoin/build.rs b/bitcoin/build.rs deleted file mode 100644 index 2b741ca0d..000000000 --- a/bitcoin/build.rs +++ /dev/null @@ -1,37 +0,0 @@ -fn main() { - let rustc = std::env::var_os("RUSTC"); - let rustc = rustc.as_ref().map(std::path::Path::new).unwrap_or_else(|| "rustc".as_ref()); - let output = std::process::Command::new(rustc) - .arg("--version") - .output() - .unwrap_or_else(|error| panic!("failed to run `{:?} --version`: {:?}", rustc, error)); - assert!(output.status.success(), "{:?} -- version returned non-zero exit code", rustc); - let stdout = String::from_utf8(output.stdout).expect("rustc produced non-UTF-8 output"); - let version_prefix = "rustc "; - if !stdout.starts_with(version_prefix) { - panic!("unexpected rustc output: {}", stdout); - } - - let version = &stdout[version_prefix.len()..]; - let end = version.find(&[' ', '-'] as &[_]).unwrap_or(version.len()); - let version = &version[..end]; - let mut version_components = version.split('.'); - let major = version_components.next().unwrap(); - assert_eq!(major, "1", "unexpected Rust major version"); - let minor = version_components - .next() - .unwrap_or("0") - .parse::() - .expect("invalid Rust minor version"); - - let msrv = std::env::var("CARGO_PKG_RUST_VERSION").unwrap(); - let mut msrv = msrv.split("."); - let msrv_major = msrv.next().unwrap(); - assert_eq!(msrv_major, "1", "unexpected Rust major version"); - let msrv_minor = msrv.next().unwrap().parse::().unwrap(); - - // print cfg for all interesting versions less than or equal to minor - for version in msrv_minor..=minor { - println!("cargo:rustc-cfg=rust_v_1_{}", version); - } -} diff --git a/internals/Cargo.toml b/internals/Cargo.toml index 7e46aa6bf..f3994febd 100644 --- a/internals/Cargo.toml +++ b/internals/Cargo.toml @@ -34,4 +34,4 @@ all-features = true rustdoc-args = ["--cfg", "docsrs"] [lints.rust] -unexpected_cfgs = { level = "deny", check-cfg = ['cfg(rust_v_1_64)'] } +unexpected_cfgs = { level = "deny" } diff --git a/internals/build.rs b/internals/build.rs index 2b741ca0d..b055def60 100644 --- a/internals/build.rs +++ b/internals/build.rs @@ -1,3 +1,7 @@ +const MAX_USED_VERSION: u64 = 80; + +use std::io; + fn main() { let rustc = std::env::var_os("RUSTC"); let rustc = rustc.as_ref().map(std::path::Path::new).unwrap_or_else(|| "rustc".as_ref()); @@ -30,8 +34,45 @@ fn main() { assert_eq!(msrv_major, "1", "unexpected Rust major version"); let msrv_minor = msrv.next().unwrap().parse::().unwrap(); - // print cfg for all interesting versions less than or equal to minor - for version in msrv_minor..=minor { - println!("cargo:rustc-cfg=rust_v_1_{}", version); - } + let out_dir = std::env::var_os("OUT_DIR").expect("missing OUT_DIR env var"); + let out_dir = std::path::PathBuf::from(out_dir); + let macro_file = std::fs::File::create(out_dir.join("rust_version.rs")).expect("failed to create rust_version.rs"); + let macro_file = io::BufWriter::new(macro_file); + write_macro(macro_file, msrv_minor, minor).expect("failed to write to rust_version.rs"); +} + +fn write_macro(mut macro_file: impl io::Write, msrv_minor: u64, minor: u64) -> io::Result<()> { + writeln!(macro_file, "/// Expands code based on Rust version this is compiled under.")?; + writeln!(macro_file, "///")?; + writeln!(macro_file, "/// Example:")?; + writeln!(macro_file, "/// ```")?; + writeln!(macro_file, "/// bitcoin_internals::rust_version! {{")?; + writeln!(macro_file, "/// if >= 1.70 {{")?; + writeln!(macro_file, "/// println!(\"This is Rust 1.70+\");")?; + writeln!(macro_file, "/// }} else {{")?; + writeln!(macro_file, "/// println!(\"This is Rust < 1.70\");")?; + writeln!(macro_file, "/// }}")?; + writeln!(macro_file, "/// }}")?; + writeln!(macro_file, "/// ```")?; + writeln!(macro_file, "///")?; + writeln!(macro_file, "/// The `else` branch is optional.")?; + writeln!(macro_file, "/// Currently only the `>=` operator is supported.")?; + writeln!(macro_file, "#[macro_export]")?; + writeln!(macro_file, "macro_rules! rust_version {{")?; + for version in msrv_minor..=minor { + writeln!(macro_file, " (if >= 1.{} {{ $($if_yes:tt)* }} $(else {{ $($if_no:tt)* }})?) => {{", version)?; + writeln!(macro_file, " $($if_yes)*")?; + writeln!(macro_file, " }};")?; + } + for version in (minor + 1)..(MAX_USED_VERSION + 1) { + writeln!(macro_file, " (if >= 1.{} {{ $($if_yes:tt)* }} $(else {{ $($if_no:tt)* }})?) => {{", version)?; + writeln!(macro_file, " $($($if_no)*)?")?; + writeln!(macro_file, " }};")?; + } + writeln!(macro_file, " (if >= $unknown:tt $($rest:tt)*) => {{")?; + writeln!(macro_file, " compile_error!(concat!(\"unknown Rust version \", stringify!($unknown)));")?; + writeln!(macro_file, " }};")?; + writeln!(macro_file, "}}")?; + writeln!(macro_file, "pub use rust_version;")?; + macro_file.flush() } diff --git a/internals/src/array_vec.rs b/internals/src/array_vec.rs index bcce012d9..f75924ba1 100644 --- a/internals/src/array_vec.rs +++ b/internals/src/array_vec.rs @@ -46,7 +46,7 @@ mod safety_boundary { // from_raw_parts is const-unstable until 1.64 cond_const! { /// Returns a reference to the underlying data. - pub const(in rust_v_1_64 = "1.64") fn as_slice(&self) -> &[T] { + pub const(in 1.64) fn as_slice(&self) -> &[T] { let ptr = &self.data as *const _ as *const T; unsafe { core::slice::from_raw_parts(ptr, self.len) } } diff --git a/internals/src/const_tools.rs b/internals/src/const_tools.rs index f475a23c7..bd4c72326 100644 --- a/internals/src/const_tools.rs +++ b/internals/src/const_tools.rs @@ -56,36 +56,34 @@ pub use concat_bytes_to_arr; #[macro_export] /// Enables const fn in specified Rust version macro_rules! cond_const { - ($($(#[$attr:meta])* $vis:vis const(in $ver:ident $(= $human_ver:literal)?) fn $name:ident$(<$($gen:tt)*>)?($($args:tt)*) $(-> $ret:ty)? $body:block)+ ) => { + ($($(#[$attr:meta])* $vis:vis const(in $version:tt) fn $name:ident$(<$($gen:tt)*>)?($($args:tt)*) $(-> $ret:ty)? $body:block)+ ) => { $( - #[cfg($ver)] - $(#[$attr])* - $( - #[doc = "\nNote: the function is only `const` in Rust "] - #[doc = $human_ver] - #[doc = "."] - )? - $vis const fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body - - #[cfg(not($ver))] - $(#[$attr])* - $vis fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + $crate::rust_version::rust_version! { + if >= $version { + $(#[$attr])* + #[doc = concat!("\nNote: the function is only `const` in Rust ", stringify!($version), ".")] + $vis const fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + } else { + $(#[$attr])* + #[doc = concat!("\nNote: the function is `const` in Rust ", stringify!($version), ".")] + $vis fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + } + } )+ }; - ($($(#[$attr:meta])* $vis:vis const(in $ver:ident $(= $human_ver:literal)?) unsafe fn $name:ident$(<$($gen:tt)*>)?($($args:tt)*) $(-> $ret:ty)? $body:block)+ ) => { + ($($(#[$attr:meta])* $vis:vis const(in $version:tt) unsafe fn $name:ident$(<$($gen:tt)*>)?($($args:tt)*) $(-> $ret:ty)? $body:block)+ ) => { $( - #[cfg($ver)] - $(#[$attr])* - $( - #[doc = "\nNote: the function is only `const` in Rust "] - #[doc = $human_ver] - #[doc = " and newer."] - )? - $vis const unsafe fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body - - #[cfg(not($ver))] - $(#[$attr])* - $vis unsafe fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + $crate::rust_version::rust_version! { + if >= $version { + $(#[$attr])* + #[doc = concat!("\nNote: the function is only `const` in Rust ", stringify!($version), ".")] + $vis const unsafe fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + } else { + $(#[$attr])* + #[doc = concat!("\nNote: the function is `const` in Rust ", stringify!($version), ".")] + $vis unsafe fn $name$(<$($gen)*>)?($($args)*) $(-> $ret)? $body + } + } )+ }; } diff --git a/internals/src/lib.rs b/internals/src/lib.rs index fb93fd50e..6ecb272a9 100644 --- a/internals/src/lib.rs +++ b/internals/src/lib.rs @@ -27,6 +27,13 @@ pub extern crate serde_json; #[cfg(feature = "test-serde")] pub extern crate bincode; +// The pub module is a workaround for strange error: +// "macro-expanded `macro_export` macros from the current crate cannot be referred to by absolute paths" +#[doc(hidden)] +pub mod rust_version { + include!(concat!(env!("OUT_DIR"), "/rust_version.rs")); +} + pub mod array_vec; pub mod const_tools; pub mod error; diff --git a/io/Cargo.toml b/io/Cargo.toml index f6747bb3b..672eb5e2e 100644 --- a/io/Cargo.toml +++ b/io/Cargo.toml @@ -18,9 +18,12 @@ default = ["std"] std = ["alloc"] alloc = [] +[dependencies] +internals = { package = "bitcoin-internals", version = "0.3.0" } + [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] [lints.rust] -unexpected_cfgs = { level = "deny", check-cfg = ['cfg(rust_v_1_72)', 'cfg(rust_v_1_73)', 'cfg(rust_v_1_75)', 'cfg(rust_v_1_78)'] } +unexpected_cfgs = { level = "deny" } diff --git a/io/build.rs b/io/build.rs deleted file mode 100644 index 2b741ca0d..000000000 --- a/io/build.rs +++ /dev/null @@ -1,37 +0,0 @@ -fn main() { - let rustc = std::env::var_os("RUSTC"); - let rustc = rustc.as_ref().map(std::path::Path::new).unwrap_or_else(|| "rustc".as_ref()); - let output = std::process::Command::new(rustc) - .arg("--version") - .output() - .unwrap_or_else(|error| panic!("failed to run `{:?} --version`: {:?}", rustc, error)); - assert!(output.status.success(), "{:?} -- version returned non-zero exit code", rustc); - let stdout = String::from_utf8(output.stdout).expect("rustc produced non-UTF-8 output"); - let version_prefix = "rustc "; - if !stdout.starts_with(version_prefix) { - panic!("unexpected rustc output: {}", stdout); - } - - let version = &stdout[version_prefix.len()..]; - let end = version.find(&[' ', '-'] as &[_]).unwrap_or(version.len()); - let version = &version[..end]; - let mut version_components = version.split('.'); - let major = version_components.next().unwrap(); - assert_eq!(major, "1", "unexpected Rust major version"); - let minor = version_components - .next() - .unwrap_or("0") - .parse::() - .expect("invalid Rust minor version"); - - let msrv = std::env::var("CARGO_PKG_RUST_VERSION").unwrap(); - let mut msrv = msrv.split("."); - let msrv_major = msrv.next().unwrap(); - assert_eq!(msrv_major, "1", "unexpected Rust major version"); - let msrv_minor = msrv.next().unwrap().parse::().unwrap(); - - // print cfg for all interesting versions less than or equal to minor - for version in msrv_minor..=minor { - println!("cargo:rustc-cfg=rust_v_1_{}", version); - } -} diff --git a/io/src/bridge.rs b/io/src/bridge.rs index 85e999167..91bad35e2 100644 --- a/io/src/bridge.rs +++ b/io/src/bridge.rs @@ -1,5 +1,6 @@ #[cfg(feature = "alloc")] use alloc::boxed::Box; +use internals::rust_version; /// A bridging wrapper providing the IO traits for types that already implement `std` IO traits. #[repr(transparent)] @@ -304,24 +305,40 @@ macro_rules! impl_our { }; } -#[cfg(rust_v_1_72)] -impl_our! { - impl Read for std::io::BufReader where R: ?Sized -} +rust_version! { + if >= 1.72 { + impl_our! { + impl Read for std::io::BufReader where R: ?Sized + } -#[cfg(not(rust_v_1_72))] -impl_our! { - impl Read for std::io::BufReader -} + impl_our! { + impl BufRead for std::io::BufReader where R: ?Sized + } -#[cfg(rust_v_1_72)] -impl_our! { - impl BufRead for std::io::BufReader where R: ?Sized -} + impl_our! { + impl Write for std::io::BufWriter where W: ?Sized + } -#[cfg(not(rust_v_1_72))] -impl_our! { - impl BufRead for std::io::BufReader + impl_our! { + impl Write for std::io::LineWriter where W: ?Sized + } + } else { + impl_our! { + impl Read for std::io::BufReader + } + + impl_our! { + impl BufRead for std::io::BufReader + } + + impl_our! { + impl Write for std::io::BufWriter + } + + impl_our! { + impl Write for std::io::LineWriter + } + } } impl std::io::Write for super::Sink { @@ -335,26 +352,6 @@ impl std::io::Write for super::Sink { fn flush(&mut self) -> std::io::Result<()> { Ok(()) } } -#[cfg(rust_v_1_72)] -impl_our! { - impl Write for std::io::BufWriter where W: ?Sized -} - -#[cfg(not(rust_v_1_72))] -impl_our! { - impl Write for std::io::BufWriter -} - -#[cfg(rust_v_1_72)] -impl_our! { - impl Write for std::io::LineWriter where W: ?Sized -} - -#[cfg(not(rust_v_1_72))] -impl_our! { - impl Write for std::io::LineWriter -} - impl_our! { impl Read for std::io::Take } @@ -399,15 +396,25 @@ impl_our! { impl BufRead for std::io::Empty } -#[cfg(rust_v_1_73)] -impl_our! { - impl Write for std::io::Empty -} +rust_version! { + if >= 1.73 { + impl_our! { + impl Write for std::io::Empty + } -// No idea why &Empty impls Write but not Read + BufRead -#[cfg(rust_v_1_73)] -impl_our! { - impl Write for &'_ std::io::Empty + // No idea why &Empty impls Write but not Read + BufRead + impl_our! { + impl Write for &'_ std::io::Empty + } + + impl_our! { + impl Read for std::sync::Arc + } + + impl_our! { + impl Write for std::sync::Arc + } + } } impl_our! { @@ -418,9 +425,12 @@ impl_our! { impl Read for std::io::Stdin } -#[cfg(rust_v_1_78)] -impl_our! { - impl Read for &'_ std::io::Stdin +rust_version! { + if >= 1.78 { + impl_our! { + impl Read for &'_ std::io::Stdin + } + } } impl_our! { @@ -463,16 +473,6 @@ impl_our! { impl Write for &'_ std::fs::File } -#[cfg(rust_v_1_73)] -impl_our! { - impl Read for std::sync::Arc -} - -#[cfg(rust_v_1_73)] -impl_our! { - impl Write for std::sync::Arc -} - impl_our! { impl Read for std::net::TcpStream } @@ -526,14 +526,16 @@ impl_our! { impl Write for &'_ std::process::ChildStdin } -#[cfg(rust_v_1_75)] -impl_our! { - impl Read for std::collections::VecDeque -} +rust_version! { + if >= 1.75 { + impl_our! { + impl Read for std::collections::VecDeque + } -#[cfg(rust_v_1_75)] -impl_our! { - impl BufRead for std::collections::VecDeque + impl_our! { + impl BufRead for std::collections::VecDeque + } + } } impl_our! {