diff --git a/support/ctor.rs b/support/ctor.rs index 72306ca6b..5ec218d93 100644 --- a/support/ctor.rs +++ b/support/ctor.rs @@ -1251,6 +1251,9 @@ pub mod macro_internal { pub use core::mem::MaybeUninit; pub use core::pin::Pin; + /// A helpfully named type alias that tells users to not directly initialize fields of this type + pub type MustUseCtorToInitialize = [u8; 0]; + /// Workaround for more_qualified_paths. /// Instead of `::Assoc { ... }`, which requires that feature, /// we can use `Identity<::Assoc> { ... }`. diff --git a/support/ctor_proc_macros.rs b/support/ctor_proc_macros.rs index 10144969e..978590ee4 100644 --- a/support/ctor_proc_macros.rs +++ b/support/ctor_proc_macros.rs @@ -449,71 +449,76 @@ impl Parse for RecursivelyPinnedArgs { /// Prevents this type from being directly created outside of this crate in safe /// code. /// -/// For enums and unit structs, this uses the `#[non_exhaustive]` attribute. -/// This leads to unfortunate error messages, but there is no other way to -/// prevent creation of an enum or a unit struct at this time. +/// We use a private field with a name and type that indicates the error: /// -/// For tuple structs, we also use `#[non_exhaustive]`, as it's no worse than -/// the alternative. Both adding a private field and adding `#[non_exhaustive]` -/// lead to indirect error messages, but `#[non_exhaustive]` is the more likely -/// of the two to ever get custom error message support. +/// ``` +/// __must_use_ctor_to_initialize: ::ctor::macro_internal::MustUseCtorToInitialize, +/// ``` /// -/// Finally, for structs with named fields, we actually *cannot* use -/// `#[non_exhaustive]`, because it would make the struct not FFI-safe, and -/// structs with named fields are specifically supported for C++ interop. -/// Instead, we use a private field with a name that indicates the error. -/// (`__must_use_ctor_to_initialize`). +/// This zero-sized type cannot be constructed in safe code, and so the compiler will prevent +/// direct initialization. /// /// Unions are not yet implemented properly. -/// -/// --- -/// -/// Note that the use of `#[non_exhaustive]` also has other effects. At the -/// least: tuple variants and tuple structs marked with `#[non_exhaustive]` -/// cannot be pattern matched using the "normal" syntax. Instead, one must use -/// curly braces. (Broken: `T(x, ..)`; woken: `T{0: x, ..}`). -/// -/// (This does not seem very intentional, and with all luck will be fixed before -/// too long.) -fn forbid_initialization(s: &mut syn::DeriveInput) { - let non_exhaustive_attr = syn::parse_quote!(#[non_exhaustive]); +fn forbid_initialization(s: &mut syn::DeriveInput, ctor: &Ident) { match &mut s.data { // TODO(b/232969667): prevent creation of unions from safe code. // (E.g. hide inside a struct.) syn::Data::Union(_) => return, syn::Data::Struct(data) => { - match &mut data.fields { - syn::Fields::Unit | syn::Fields::Unnamed(_) => { - s.attrs.insert(0, non_exhaustive_attr); - } - syn::Fields::Named(fields) => { - // insert at the front in case the last field is !Sized. - fields.named.insert( - 0, - syn::Field { - attrs: vec![], - vis: syn::Visibility::Inherited, - mutability: syn::FieldMutability::None, - // TODO(jeanpierreda): better hygiene: work even if a field has the same name. - ident: Some(Ident::new(FIELD_FOR_MUST_USE_CTOR, Span::call_site())), - colon_token: Some(::default()), - ty: syn::parse_quote!([u8; 0]), - }, - ); - } - } + forbid_initialization_fields(&mut data.fields, ctor); } syn::Data::Enum(e) => { - // Enums can't have private fields. Instead, we need to add #[non_exhaustive] to - // every variant -- this makes it impossible to construct the - // variants. for variant in &mut e.variants { - variant.attrs.insert(0, non_exhaustive_attr.clone()); + forbid_initialization_fields(&mut variant.fields, ctor); } } } } +fn forbid_initialization_fields(fields: &mut syn::Fields, ctor: &Ident) { + match fields { + syn::Fields::Unit => { + // TODO(jeanpierreda): better hygiene: work even if a field has the same name. + let ident = Ident::new(FIELD_FOR_MUST_USE_CTOR, Span::call_site()); + *fields = syn::Fields::Named(syn::parse_quote!({ + #ident: ::#ctor::macro_internal::MustUseCtorToInitialize, + })); + } + syn::Fields::Unnamed(fields) => { + // Push to the back to keep the indices lining up with the types. If we inserted to the + // front, the indices would shift which might cause extremely subtle bugs if someone + // tried to use FnCtor and used `std::mem::offset_of!(TupleStruct, N)`. For example, + // if N = 1, they might think they're getting the offset of the second field, but they'd + // actually get the offset of the first field (because N = 0 is the hidden field). + // The only downside to this is that users will get a horribly confusing error if they + // try to have their type coerce to a DST. + fields.unnamed.push(syn::Field { + attrs: vec![], + vis: syn::Visibility::Inherited, + mutability: syn::FieldMutability::None, + ident: None, + colon_token: None, + ty: syn::parse_quote!(::#ctor::macro_internal::MustUseCtorToInitialize), + }); + } + syn::Fields::Named(fields) => { + // insert at the front in case the last field is !Sized. + fields.named.insert( + 0, + syn::Field { + attrs: vec![], + vis: syn::Visibility::Inherited, + mutability: syn::FieldMutability::None, + // TODO(jeanpierreda): better hygiene: work even if a field has the same name. + ident: Some(Ident::new(FIELD_FOR_MUST_USE_CTOR, Span::call_site())), + colon_token: Some(::default()), + ty: syn::parse_quote!(::#ctor::macro_internal::MustUseCtorToInitialize), + }, + ); + } + } +} + /// `#[recursively_pinned]` pins every field, similar to `#[pin_project]`, and /// marks the struct `!Unpin`. /// @@ -631,11 +636,9 @@ fn forbid_initialization(s: &mut syn::DeriveInput) { /// Recursively pinned types cannot be created directly in safe code, as they /// are pinned from the very moment of their creation. /// -/// This is prevented either using `#[non_exhaustive]` or using a private field, -/// depending on the type in question. For example, enums use -/// `#[non_exhaustive]`, and structs with named fields use a private field named -/// `__must_use_ctor_to_initialize`. This can lead to confusing error messages, -/// so watch out! +/// This is prevented using a private field of type +/// `ctor::macro_internal::MustUseCtorToInitialize`. This can lead to confusing +/// error messages, so watch out! /// /// ## Pin-projection /// @@ -671,7 +674,11 @@ fn forbid_initialization(s: &mut syn::DeriveInput) { /// /// Structs, enums, and unions are all supported. However, unions do not receive /// a `project_{pin,ref}` methods, as there is no way to implement pin projection for -/// unions. (One cannot know which field is active.) +/// unions (one cannot know which field is active). Lastly, although DST structs are +/// supported, DST tuple structs are not supported. This is because the unsized field must be +/// the last field in the struct, but the insertion of a hidden `MustUseCtorToInitialize` field +/// would break this requirement, and inserting it at the front would cause surprising behavior +/// with respect to offset calculations. #[proc_macro_attribute] pub fn recursively_pinned(args: TokenStream, item: TokenStream) -> TokenStream { match recursively_pinned_impl(args.into(), item.into()) { @@ -710,7 +717,7 @@ fn recursively_pinned_impl( // type is locally defined within a narrow scope. ctor_initialized_input.ident = syn::Ident::new(&format!("__CrubitCtor{name}"), name.span()); let ctor_initialized_name = &ctor_initialized_input.ident; - forbid_initialization(&mut input); + forbid_initialization(&mut input, &ctor); let (input_impl_generics, input_type_generics, input_where_clause) = input.generics.split_for_impl(); @@ -789,7 +796,7 @@ mod test { quote! { #[repr(C)] struct S { - __must_use_ctor_to_initialize: [u8; 0], + __must_use_ctor_to_initialize: ::ctor::macro_internal::MustUseCtorToInitialize, x: i32 } } @@ -841,10 +848,10 @@ mod test { quote! { #[repr(C)] enum E { - #[non_exhaustive] - A, - #[non_exhaustive] - B(i32), + A { + __must_use_ctor_to_initialize: ::ctor::macro_internal::MustUseCtorToInitialize, + }, + B(i32, ::ctor::macro_internal::MustUseCtorToInitialize), } } ); diff --git a/support/ctor_proc_macros_e2e_test.rs b/support/ctor_proc_macros_e2e_test.rs index 7771f271c..869d1f9d0 100644 --- a/support/ctor_proc_macros_e2e_test.rs +++ b/support/ctor_proc_macros_e2e_test.rs @@ -125,9 +125,9 @@ fn test_derive_move_and_assign_via_copy_with_generics() { fn test_recursively_pinned_unit_struct() { #[::ctor::recursively_pinned] struct S; - let _ = Box::pin(S).as_mut().project_pin(); + let _ = ::ctor::emplace!(::ctor::ctor!(S)).as_mut().project_pin(); assert_eq!(::std::mem::size_of::<::ctor::project_pin_type!(S)>(), 0); - let _ = Box::pin(S).as_ref().project_ref(); + let _ = ::ctor::emplace!(::ctor::ctor!(S)).as_ref().project_ref(); assert_eq!(::std::mem::size_of::<::ctor::project_ref_type!(S)>(), 0); } @@ -135,9 +135,7 @@ fn test_recursively_pinned_unit_struct() { fn test_recursively_pinned_fieldless_struct() { #[::ctor::recursively_pinned] struct S {} - let mut b = Box::pin(S { - __must_use_ctor_to_initialize: [], // for tests only! - }); + let mut b = ::ctor::emplace!(::ctor::ctor!(S {})); let _ = b.as_mut().project_pin(); assert_eq!(::std::mem::size_of::<::ctor::project_pin_type!(S)>(), 0); let _ = b.as_ref().project_ref(); @@ -148,32 +146,22 @@ fn test_recursively_pinned_fieldless_struct() { fn test_recursively_pinned_fieldless_tuple_struct() { #[::ctor::recursively_pinned] struct S(); - let _ = Box::pin(S()).as_mut().project_pin(); + let _ = ::ctor::emplace!(::ctor::ctor!(S())).as_mut().project_pin(); assert_eq!(::std::mem::size_of::<::ctor::project_pin_type!(S)>(), 0); - let _ = Box::pin(S()).as_ref().project_ref(); + let _ = ::ctor::emplace!(::ctor::ctor!(S())).as_ref().project_ref(); assert_eq!(::std::mem::size_of::<::ctor::project_ref_type!(S)>(), 0); } -#[gtest] -fn test_recursively_pinned_fieldless_enum() { - #[::ctor::recursively_pinned] - enum E { - A, - } - let <::ctor::project_pin_type!(E)>::A = Box::pin(E::A).as_mut().project_pin(); - assert_eq!(::std::mem::size_of::<::ctor::project_pin_type!(E)>(), 0); - let <::ctor::project_ref_type!(E)>::A = Box::pin(E::A).as_ref().project_ref(); - assert_eq!(::std::mem::size_of::<::ctor::project_ref_type!(E)>(), 0); -} - #[gtest] fn test_recursively_pinned_in_module() { mod submodule { #[::ctor::recursively_pinned] pub struct S; } - let _: ::ctor::project_pin_type!(submodule::S) = Box::pin(submodule::S).as_mut().project_pin(); - let _: ::ctor::project_ref_type!(submodule::S) = Box::pin(submodule::S).as_ref().project_ref(); + let _: ::ctor::project_pin_type!(submodule::S) = + ::ctor::emplace!(::ctor::ctor!(submodule::S)).as_mut().project_pin(); + let _: ::ctor::project_ref_type!(submodule::S) = + ::ctor::emplace!(::ctor::ctor!(submodule::S)).as_ref().project_ref(); } #[gtest] @@ -182,10 +170,7 @@ fn test_recursively_pinned_struct() { struct S { x: i32, } - let mut b = Box::pin(S { - x: 42, - __must_use_ctor_to_initialize: [], // for tests only! - }); + let mut b = ::ctor::emplace!(::ctor::ctor!(S { x: 42 })); let _: ::std::pin::Pin<&mut i32> = b.as_mut().project_pin().x; let _: ::std::pin::Pin<&i32> = b.as_ref().project_ref().x; @@ -195,49 +180,9 @@ fn test_recursively_pinned_struct() { fn test_recursively_pinned_tuple_struct() { #[::ctor::recursively_pinned] struct S(i32); - let _: ::std::pin::Pin<&mut i32> = Box::pin(S(42)).as_mut().project_pin().0; - let _: ::std::pin::Pin<&i32> = Box::pin(S(42)).as_ref().project_ref().0; -} - -// TODO(b/331688163): remove this workaround. -type Identity = T; - -#[gtest] -fn test_recursively_pinned_enum_struct() { - #[::ctor::recursively_pinned] - enum E { - A { x: i32 }, - } - let mut b = Box::pin(E::A { x: 42 }); - match b.as_mut().project_pin() { - Identity::<::ctor::project_pin_type!(E)>::A { x } => { - let _: ::std::pin::Pin<&mut i32> = x; - } - } - match b.as_ref().project_ref() { - Identity::<::ctor::project_ref_type!(E)>::A { x } => { - let _: ::std::pin::Pin<&i32> = x; - } - } -} - -#[gtest] -fn test_recursively_pinned_enum_tuple() { - #[::ctor::recursively_pinned] - enum E { - A(i32), - } - let mut b = Box::pin(E::A(42)); - match b.as_mut().project_pin() { - Identity::<::ctor::project_pin_type!(E)>::A(x) => { - let _: ::std::pin::Pin<&mut i32> = x; - } - } - match b.as_ref().project_ref() { - Identity::<::ctor::project_ref_type!(E)>::A(x) => { - let _: ::std::pin::Pin<&i32> = x; - } - } + let _: ::std::pin::Pin<&mut i32> = + ::ctor::emplace!(::ctor::ctor!(S(42))).as_mut().project_pin().0; + let _: ::std::pin::Pin<&i32> = ::ctor::emplace!(::ctor::ctor!(S(42))).as_ref().project_ref().0; } #[gtest] @@ -252,11 +197,8 @@ fn test_recursively_pinned_generic() { /// the works. _phantom: ::std::marker::PhantomData<&'proj &'proj_2 &'proj_4 T>, } - let mut b = Box::pin(S:: { - x: 42, - _phantom: ::std::marker::PhantomData, - __must_use_ctor_to_initialize: [], // for tests only! - }); + let mut b = + ::ctor::emplace!(::ctor::ctor!(S:: { x: 42, _phantom: ::std::marker::PhantomData })); let _: ::std::pin::Pin<&mut i32> = b.as_mut().project_pin().x; let _: ::std::pin::Pin<&i32> = b.as_ref().project_ref().x; } @@ -365,7 +307,9 @@ fn test_pinned_drop() { } let called_drop = Rc::new(Cell::new(false)); - let _ = DropStruct(called_drop.clone()); + { + let _ = ::ctor::emplace!(::ctor::ctor!(DropStruct(called_drop.clone()))); + } assert!(called_drop.get(), "PinnedDrop::pinned_drop was not called"); } diff --git a/support/ctor_rename_test.rs b/support/ctor_rename_test.rs index fc014d092..ccbf44488 100644 --- a/support/ctor_rename_test.rs +++ b/support/ctor_rename_test.rs @@ -49,6 +49,6 @@ fn test_derive_move_and_assign_via_copy() { fn test_recursively_pinned_unit_struct() { #[renamed_ctor::recursively_pinned(crate = renamed_ctor)] struct S; - let _ = Box::pin(S).as_mut().project_pin(); + let _ = ::renamed_ctor::emplace!(::renamed_ctor::ctor!(S)).as_mut().project_pin(); assert_eq!(std::mem::size_of::(), 0); }