diff --git a/src/lib.rs b/src/lib.rs index b488c8c8ff3..3b289282838 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4758,6 +4758,31 @@ fn mut_from_prefix_suffix( /// /// [type layout]: https://doc.rust-lang.org/reference/type-layout.html /// +/// # Unions +/// +/// Currently, union bit validity is [up in the air][union-validity], and so +/// zerocopy does not support `#[derive(IntoBytes)]` on unions by default. +/// However, implementing `IntoBytes` on a union type is likely sound on all +/// existing Rust toolchains - it's just that it may become unsound in the +/// future. You can opt-in to `#[derive(IntoBytes)]` support on unions by +/// passing the unstable `zerocopy_derive_union_into_bytes` cfg: +/// +/// ```shell +/// $ RUSTFLAGS='--cfg zerocopy_derive_union_into_bytes' cargo build +/// ``` +/// +/// We make no stability guarantees regarding this cfg, and may remove it at any +/// point. +/// +/// We are actively working with Rust to stabilize the necessary language +/// guarantees to support this in a forwards-compatible way, which will enable +/// us to remove the cfg gate. As part of this effort, we need to know how much +/// demand there is for this feature. If you would like to use `IntoBytes` on +/// unions, [please let us know][discussion]. +/// +/// [union-validity]: https://github.com/rust-lang/unsafe-code-guidelines/issues/438 +/// [discussion]: https://github.com/google/zerocopy/discussions/1802 +/// /// # Analysis /// /// *This section describes, roughly, the analysis performed by this derive to @@ -4821,15 +4846,6 @@ pub use zerocopy_derive::IntoBytes; /// ... /// # */ /// } -/// -/// #[derive(IntoBytes)] -/// #[repr(C)] -/// union MyUnion { -/// # variant: u8, -/// # /* -/// ... -/// # */ -/// } /// ``` /// /// This derive performs a sophisticated, compile-time safety analysis to diff --git a/tools/cargo-zerocopy/src/main.rs b/tools/cargo-zerocopy/src/main.rs index 17eddbcbc8c..ec179b54f34 100644 --- a/tools/cargo-zerocopy/src/main.rs +++ b/tools/cargo-zerocopy/src/main.rs @@ -164,10 +164,11 @@ fn install_toolchain_or_exit(versions: &Versions, name: &str) -> Result<(), Erro } fn get_rustflags(name: &str) -> &'static str { + // See #1792 for context on zerocopy_derive_union_into_bytes. if name == "nightly" { - "--cfg __ZEROCOPY_INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS " + "--cfg __ZEROCOPY_INTERNAL_USE_ONLY_NIGHTLY_FEATURES_IN_TESTS --cfg zerocopy_derive_union_into_bytes " } else { - "" + "--cfg zerocopy_derive_union_into_bytes " } } diff --git a/zerocopy-derive/Cargo.toml b/zerocopy-derive/Cargo.toml index 9364ff53540..2659ae3cda7 100644 --- a/zerocopy-derive/Cargo.toml +++ b/zerocopy-derive/Cargo.toml @@ -22,6 +22,10 @@ repository = "https://github.com/google/zerocopy" # [1] https://github.com/rust-lang/crater exclude = [".*", "tests/enum_from_bytes.rs", "tests/ui-nightly/enum_from_bytes_u16_too_few.rs.disabled"] +[lints.rust] +# See #1792 for more context. +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(zerocopy_derive_union_into_bytes)'] } + [lib] proc-macro = true @@ -32,9 +36,6 @@ syn = { version = "2.0.46", features = ["full"] } [dev-dependencies] dissimilar = "1.0.9" -# We don't use this directly, but trybuild does. On the MSRV toolchain, the -# version resolver fails to select any version for once_cell unless we -# depend on it directly. once_cell = "=1.9" # This is the latest version which is compatible with `syn` 2.0.46, which we pin # to in CI for MSRV compatibility reasons. diff --git a/zerocopy-derive/src/lib.rs b/zerocopy-derive/src/lib.rs index b47474fc123..ea5275832ce 100644 --- a/zerocopy-derive/src/lib.rs +++ b/zerocopy-derive/src/lib.rs @@ -934,6 +934,17 @@ fn derive_into_bytes_enum(ast: &DeriveInput, enm: &DataEnum) -> Result Result { + // See #1792 for more context. + let cfg_compile_error = quote!( + const _: () = { + #[cfg(not(zerocopy_derive_union_into_bytes))] + ::zerocopy::util::macro_util::core_reexport::compile_error!( + "requires --cfg zerocopy_derive_union_into_bytes; +please let us know you use this feature: https://github.com/google/zerocopy/discussions/1802" + ); + }; + ); + // TODO(#10): Support type parameters. if !ast.generics.params.is_empty() { return Err(Error::new(Span::call_site(), "unsupported on types with type parameters")); @@ -951,7 +962,7 @@ fn derive_into_bytes_union(ast: &DeriveInput, unn: &DataUnion) -> Result Result, + { + fn only_derive_is_allowed_to_implement_this_trait() {} + } + } no_build + } +} + #[test] fn test_unaligned() { test! { diff --git a/zerocopy-derive/tests/trybuild.rs b/zerocopy-derive/tests/trybuild.rs index 8506d740d9c..9772eeb2fbd 100644 --- a/zerocopy-derive/tests/trybuild.rs +++ b/zerocopy-derive/tests/trybuild.rs @@ -6,11 +6,11 @@ // This file may not be copied, modified, or distributed except according to // those terms. +use once_cell::sync::OnceCell; +use std::{env, mem, sync::Mutex}; use testutil::set_rustflags_w_warnings; -#[test] -#[cfg_attr(miri, ignore)] -fn ui() { +fn test(subdir: &str) { let version = testutil::ToolchainVersion::extract_from_pwd().unwrap(); // See the doc comment on this method for an explanation of what this does // and why we store source files in different directories. @@ -21,5 +21,40 @@ fn ui() { set_rustflags_w_warnings(); let t = trybuild::TestCases::new(); - t.compile_fail(format!("tests/{}/*.rs", source_files_dirname)); + t.compile_fail(format!("tests/{}/{}/*.rs", source_files_dirname, subdir)); +} + +// NOTE: `OnceCell` is required because const initialization of `Mutex`es is not +// supported on our MSRV. +static ENV_MTX: OnceCell> = OnceCell::new(); + +#[test] +#[cfg_attr(miri, ignore)] +fn ui() { + let guard = ENV_MTX.get_or_init(Default::default).lock().unwrap(); + test(""); + mem::drop(guard); +} + +#[test] +#[cfg_attr(miri, ignore)] +fn ui_union_into_bytes_cfg() { + // This tests the behavior when `--cfg zerocopy_derive_union_into_bytes` is + // not present, so remove it. If this logic is wrong, that's fine - it will + // exhibit as a test failure that we can debug at that point. + let rustflags = env::var("RUSTFLAGS").unwrap(); + let rustflags = rustflags.replace("--cfg zerocopy_derive_union_into_bytes", ""); + + let guard = ENV_MTX.get_or_init(Default::default).lock().unwrap(); + // SAFETY: Thanks to holding `ENV_MTX`, we guarantee that the `ui` test is + // not running in this critical section. It's still possible that the test + // framework has spawned other threads that are concurrently accessing env + // vars, but we can't do anything about that. + #[allow(unused_unsafe)] // `set_var` is safe on our MSRV. + unsafe { + env::set_var("RUSTFLAGS", rustflags) + }; + mem::drop(guard); + + test("union_into_bytes_cfg"); } diff --git a/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.rs b/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.rs new file mode 120000 index 00000000000..66ef0de5bec --- /dev/null +++ b/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.rs @@ -0,0 +1 @@ +../../ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs \ No newline at end of file diff --git a/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.stderr b/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.stderr new file mode 100644 index 00000000000..d25c238f6ea --- /dev/null +++ b/zerocopy-derive/tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.stderr @@ -0,0 +1,8 @@ +error: requires --cfg zerocopy_derive_union_into_bytes; +please let us know you use this feature: https://github.com/google/zerocopy/discussions/1802 + --> tests/ui-msrv/union_into_bytes_cfg/union_into_bytes_cfg.rs:20:10 + | +20 | #[derive(IntoBytes)] + | ^^^^^^^^^ + | + = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs b/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs new file mode 100644 index 00000000000..280f05d4103 --- /dev/null +++ b/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs @@ -0,0 +1,26 @@ +// Copyright 2024 The Fuchsia Authors +// +// Licensed under a BSD-style license , Apache License, Version 2.0 +// , or the MIT +// license , at your option. +// This file may not be copied, modified, or distributed except according to +// those terms. + +//! See: https://github.com/google/zerocopy/issues/553 +//! zerocopy must still allow derives of deprecated types. +//! This test has a hand-written impl of a deprecated type, and should result in a compilation +//! error. If zerocopy does not tack an allow(deprecated) annotation onto its impls, then this +//! test will fail because more than one compile error will be generated. +#![deny(deprecated)] + +extern crate zerocopy; + +use zerocopy::IntoBytes; + +#[derive(IntoBytes)] +#[repr(C)] +union Foo { + a: u8, +} + +fn main() {} diff --git a/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.stderr b/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.stderr new file mode 100644 index 00000000000..d687c22a885 --- /dev/null +++ b/zerocopy-derive/tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.stderr @@ -0,0 +1,8 @@ +error: requires --cfg zerocopy_derive_union_into_bytes; + please let us know you use this feature: https://github.com/google/zerocopy/discussions/1802 + --> tests/ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs:20:10 + | +20 | #[derive(IntoBytes)] + | ^^^^^^^^^ + | + = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.rs b/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.rs new file mode 120000 index 00000000000..66ef0de5bec --- /dev/null +++ b/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.rs @@ -0,0 +1 @@ +../../ui-nightly/union_into_bytes_cfg/union_into_bytes_cfg.rs \ No newline at end of file diff --git a/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.stderr b/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.stderr new file mode 100644 index 00000000000..e4e35d6256f --- /dev/null +++ b/zerocopy-derive/tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.stderr @@ -0,0 +1,8 @@ +error: requires --cfg zerocopy_derive_union_into_bytes; + please let us know you use this feature: https://github.com/google/zerocopy/discussions/1802 + --> tests/ui-stable/union_into_bytes_cfg/union_into_bytes_cfg.rs:20:10 + | +20 | #[derive(IntoBytes)] + | ^^^^^^^^^ + | + = note: this error originates in the derive macro `IntoBytes` (in Nightly builds, run with -Z macro-backtrace for more info)