diff --git a/ffi-proc-macros/src/lib.rs b/ffi-proc-macros/src/lib.rs index 015c1592f..5d28b904e 100644 --- a/ffi-proc-macros/src/lib.rs +++ b/ffi-proc-macros/src/lib.rs @@ -80,10 +80,22 @@ fn bool_to_boolean(b: bool) -> TokenStream2 { } else { quote! { False } }; - quote! { crate::handle::#name } + quote! { delta_kernel_ffi::handle::#name } } /// Macro for conveniently deriving a `delta_kernel_ffi::handle::HandleDescriptor`. +/// +/// When targeting a struct, it is invoked with three arguments: +/// ```ignore +/// #[handle_descriptor(target = Foo, mutable = false. sized = true)] +/// pub struct SharedFoo; +/// ``` +/// +/// When targeting a trait, two arguments suffice (`sized = false` is implied): +/// ```ignore +/// #[handle_descriptor(target = dyn Bar, mutable = true)] +/// pub struct MutableBar; +/// ``` #[proc_macro_attribute] pub fn handle_descriptor(attr: TokenStream, item: TokenStream) -> TokenStream { let descriptor_params = parse_macro_input!(attr as HandleDescriptorParams); @@ -105,7 +117,7 @@ pub fn handle_descriptor(attr: TokenStream, item: TokenStream) -> TokenStream { // Inject a single unconstructible member to produce a NZST let ident = &st.ident; - let new_struct = quote! { struct #ident(crate::handle::Unconstructable); }; + let new_struct = quote! { struct #ident(delta_kernel_ffi::handle::Unconstructable); }; let new_struct = new_struct.into(); let new_struct = parse_macro_input!(new_struct as ItemStruct); st.fields = new_struct.fields; @@ -116,7 +128,7 @@ pub fn handle_descriptor(attr: TokenStream, item: TokenStream) -> TokenStream { let target = &descriptor_params.target; let descriptor_impl = quote! { #[automatically_derived] - impl crate::handle::HandleDescriptor for #ident { + impl delta_kernel_ffi::handle::HandleDescriptor for #ident { type Target = #target; type Mutable = #mutable; type Sized = #sized; diff --git a/ffi/Cargo.toml b/ffi/Cargo.toml index becff06e9..f322257aa 100644 --- a/ffi/Cargo.toml +++ b/ffi/Cargo.toml @@ -10,7 +10,7 @@ version.workspace = true build = "build.rs" [lib] -crate-type = ["cdylib", "staticlib"] +crate-type = ["lib", "cdylib", "staticlib"] [dependencies] tracing = "0.1" @@ -35,6 +35,7 @@ libc = "0.2.147" [dev-dependencies] rand = "0.8.5" +trybuild = "1.0" [features] default = ["default-engine"] @@ -45,3 +46,4 @@ default-engine = [ "arrow-schema", ] sync-engine = ["delta_kernel/sync-engine"] +developer-visibility = [] \ No newline at end of file diff --git a/ffi/src/handle.rs b/ffi/src/handle.rs index 6fd6a4f64..81cba6b93 100644 --- a/ffi/src/handle.rs +++ b/ffi/src/handle.rs @@ -3,7 +3,7 @@ //! //! Creating a [`Handle`] always implies some kind of ownership transfer. A mutable handle takes //! ownership of the object itself (analagous to [`Box`]), while a non-mutable (shared) handle -//! takes ownership of a shared reference to the object (analagous to [`Arc`]). Thus, a created +//! takes ownership of a shared reference to the object (analagous to [`std::sync::Arc`]). Thus, a created //! handle remains [valid][Handle#Validity], and its underlying object remains accessible, until the //! handle is explicitly dropped or consumed. Dropping a mutable handle always drops the underlying //! object as well; dropping a shared handle only drops the underlying object if the handle was the @@ -18,20 +18,33 @@ //! advised to maintain "unique pointer" semantics for mutable handles. //! //! NOTE: While shared handles could conceptually impl [`Clone`], cloning would require unsafe code -//! and so we can't actually implement the trait. Use [`CloneHandle::clone_handle`] instead. +//! and so we can't actually implement the trait. Use [`Handle::clone_handle`] instead. /// Describes the kind of handle a given opaque pointer type represents. /// /// It is not normally necessary to implement this trait directly; instead, use the provided -/// attribute macro `#[handle_descriptor]` to generate the implementation automatically, e.g. +/// attribute macro [`#[handle_descriptor]`][delta_kernel_ffi_macros::handle_descriptor] to generate +/// the implementation automatically, e.g. /// ``` +/// # use delta_kernel_ffi_macros::handle_descriptor; +/// # use delta_kernel_ffi::handle::Handle; /// pub struct Foo { /// x: usize, /// } /// -/// #[handle_descriptor(target = "Foo", mutable = true, sized = true)] +/// #[handle_descriptor(target = Foo, mutable = true, sized = true)] /// pub struct MutableFoo; +/// +/// pub trait Bar: Send {} +/// +/// #[handle_descriptor(target = dyn Bar, mutable = false)] +/// pub struct SharedBar; +/// /// ``` +/// +/// NOTE: For obscure rust generic type constraint reasons, a `HandleDescriptor` must specifically +/// state whether the target type is [`Sized`]. However, two arguments suffice for trait targets, +/// because `sized=true` is implied. pub trait HandleDescriptor { /// The true type of the handle's underlying object type Target: ?Sized + Send; @@ -53,7 +66,17 @@ mod private { use super::*; /// Represents an object that crosses the FFI boundary and which outlives the scope that created - /// it. It can be passed freely between rust code and external code. + /// it. It can be passed freely between rust code and external code. The + /// + /// An accompanying [`HandleDescriptor`] trait defines the behavior of each handle type: + /// + /// * The true underlying ("target") type the handle represents. For safety reasons, target type + /// must always be [`Send`]. + /// + /// * Mutable (`Box`-like) vs. shared (`Arc`-like). For safety reasons, the target type of a + /// shared handle must always be [`Send`]+[`Sync`]. + /// + /// * Sized vs. unsized. Sized types allow handle operations to be implemented more efficiently. /// /// # Validity /// @@ -512,12 +535,18 @@ mod tests { } unsafe impl Send for NotSync {} - #[handle_descriptor(target=NotSync, mutable=false, sized=true)] - pub struct SharedNotSync; - #[handle_descriptor(target=NotSync, mutable=true, sized=true)] pub struct MutNotSync; + // Because tests compile as binaries against packages, this test can only run correctly if we + // use developer-visibility to make mod handle public. Otherwise it's inaccessible for testing. + #[test] + #[cfg(feature = "developer-visibility")] + fn invalid_handle_code() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/invalid-handle-code/*.rs"); + } + #[test] fn test_handle_use_cases_compile() { let s = NotSync { @@ -526,11 +555,6 @@ mod tests { let h: Handle = Box::new(s).into(); unsafe { h.drop_handle() }; - // error[E0277]: `*mut u32` cannot be shared between threads safely - // let s = NotSync { ptr: std::ptr::null_mut() }; - // let h: Handle = Arc::new(s).into(); - // unsafe { h.drop_handle() }; - let f = Foo { x: rand::random::(), y: rand::random::().to_string(), @@ -540,18 +564,8 @@ mod tests { let r = unsafe { h.as_mut() }; assert_eq!(format!("{r:?}"), s); - // error[E0599]: the method `clone_as_arc` exists for struct `Handle`, - // but its trait bounds were not satisfied - //let _ = unsafe { h.clone_as_arc() }; - unsafe { h.drop_handle() }; - // error[E0382]: borrow of moved value: `h` - // let _ = unsafe { h.as_mut() }; - - // error[E0451]: field `ptr` of struct `Handle` is private - // let h = Handle::{ ptr: std::ptr::null() }; - let b = Bar { x: rand::random::(), y: rand::random::().to_string(), @@ -561,17 +575,10 @@ mod tests { let r = unsafe { h.as_ref() }; assert_eq!(format!("{r:?}"), s); - // error[E0599]: the method `as_mut` exists for struct `Handle`, - // but its trait bounds were not satisfied - // let r = unsafe { h.as_mut() }; - let r = unsafe { h.clone_as_arc() }; assert_eq!(format!("{r:?}"), s); unsafe { h.drop_handle() }; - // error[E0382]: borrow of moved value: `h` - // let _ = unsafe { h.as_ref() }; - let b = Bar { x: rand::random::(), y: rand::random::().to_string(), @@ -605,10 +612,6 @@ mod tests { unsafe { h.drop_handle() }; - // error[E0599]: the method `clone_as_arc` exists for struct `Handle`, - // but its trait bounds were not satisfied - //let _ = unsafe { h.clone_as_arc() }; - let r = unsafe { h2.as_ref() }; assert_eq!(r.squawk(), s2); unsafe { h2.drop_handle() }; diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index ed913e226..493170cde 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -16,9 +16,21 @@ use delta_kernel::snapshot::Snapshot; use delta_kernel::{DeltaResult, Engine, Error, Table}; use delta_kernel_ffi_macros::handle_descriptor; +// cbindgen doesn't understand our use of feature flags here, and by default it parses `mod handle` +// twice. So we tell it to ignore one of the declarations to avoid double-definition errors. +/// cbindgen:ignore +#[cfg(feature = "developer-visibility")] +pub mod handle; +#[cfg(not(feature = "developer-visibility"))] pub(crate) mod handle; + use handle::Handle; +// The handle_descriptor macro needs this, because it needs to emit fully qualified type names. THe +// actual prod code could use `crate::`, but doc tests can't because they're not "inside" the crate. +// relies on `crate::` +extern crate self as delta_kernel_ffi; + pub mod scan; pub(crate) type NullableCvoid = Option>; @@ -66,8 +78,9 @@ impl Iterator for EngineIterator { /// references to the slice or its data that could outlive the function call. /// /// ``` -/// fn wants_slice(slice: KernelStringSlice) { ... } -/// let msg = String::from(...); +/// # use delta_kernel_ffi::KernelStringSlice; +/// fn wants_slice(slice: KernelStringSlice) { } +/// let msg = String::from("hello"); /// wants_slice(msg.into()); /// ``` #[repr(C)] diff --git a/ffi/tests/invalid-handle-code/double-mut-reference.rs b/ffi/tests/invalid-handle-code/double-mut-reference.rs new file mode 100644 index 000000000..4712e6cb3 --- /dev/null +++ b/ffi/tests/invalid-handle-code/double-mut-reference.rs @@ -0,0 +1,16 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; + +pub struct Foo(u32); + +#[handle_descriptor(target=Foo, mutable=true, sized=true)] +pub struct MutFoo; + +fn main() { + let s = Foo(0); + let mut h: Handle = Box::new(s).into(); + let r = unsafe { h.as_mut() }; + let _ = unsafe { h.as_mut() }; + let _ = unsafe { h.as_ref() }; + r.0 = 1; +} diff --git a/ffi/tests/invalid-handle-code/double-mut-reference.stderr b/ffi/tests/invalid-handle-code/double-mut-reference.stderr new file mode 100644 index 000000000..4504209b7 --- /dev/null +++ b/ffi/tests/invalid-handle-code/double-mut-reference.stderr @@ -0,0 +1,21 @@ +error[E0499]: cannot borrow `h` as mutable more than once at a time + --> tests/invalid-handle-code/double-mut-reference.rs:13:22 + | +12 | let r = unsafe { h.as_mut() }; + | - first mutable borrow occurs here +13 | let _ = unsafe { h.as_mut() }; + | ^ second mutable borrow occurs here +14 | let _ = unsafe { h.as_ref() }; +15 | r.0 = 1; + | ------- first borrow later used here + +error[E0502]: cannot borrow `h` as immutable because it is also borrowed as mutable + --> tests/invalid-handle-code/double-mut-reference.rs:14:22 + | +12 | let r = unsafe { h.as_mut() }; + | - mutable borrow occurs here +13 | let _ = unsafe { h.as_mut() }; +14 | let _ = unsafe { h.as_ref() }; + | ^ immutable borrow occurs here +15 | r.0 = 1; + | ------- mutable borrow later used here diff --git a/ffi/tests/invalid-handle-code/moved-from-handle.rs b/ffi/tests/invalid-handle-code/moved-from-handle.rs new file mode 100644 index 000000000..85a376463 --- /dev/null +++ b/ffi/tests/invalid-handle-code/moved-from-handle.rs @@ -0,0 +1,15 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; + +pub struct Foo(u32); + +#[handle_descriptor(target=Foo, mutable=true, sized=true)] +pub struct MutFoo; + +fn main() { + let s = Foo(0); + let mut h: Handle = Box::new(s).into(); + unsafe { h.drop_handle() }; + let _ = unsafe { h.into_inner() }; + let _ = unsafe { h.as_mut() }; +} diff --git a/ffi/tests/invalid-handle-code/moved-from-handle.stderr b/ffi/tests/invalid-handle-code/moved-from-handle.stderr new file mode 100644 index 000000000..6d7e3e032 --- /dev/null +++ b/ffi/tests/invalid-handle-code/moved-from-handle.stderr @@ -0,0 +1,32 @@ +error[E0382]: use of moved value: `h` + --> tests/invalid-handle-code/moved-from-handle.rs:13:22 + | +11 | let mut h: Handle = Box::new(s).into(); + | ----- move occurs because `h` has type `Handle`, which does not implement the `Copy` trait +12 | unsafe { h.drop_handle() }; + | ------------- `h` moved due to this method call +13 | let _ = unsafe { h.into_inner() }; + | ^ value used here after move + | +note: `Handle::::drop_handle` takes ownership of the receiver `self`, which moves `h` + --> src/handle.rs + | + | pub unsafe fn drop_handle(self) { + | ^^^^ + +error[E0382]: borrow of moved value: `h` + --> tests/invalid-handle-code/moved-from-handle.rs:14:22 + | +11 | let mut h: Handle = Box::new(s).into(); + | ----- move occurs because `h` has type `Handle`, which does not implement the `Copy` trait +12 | unsafe { h.drop_handle() }; +13 | let _ = unsafe { h.into_inner() }; + | ------------ `h` moved due to this method call +14 | let _ = unsafe { h.as_mut() }; + | ^ value borrowed here after move + | +note: `Handle::::into_inner` takes ownership of the receiver `self`, which moves `h` + --> src/handle.rs + | + | pub unsafe fn into_inner(self) -> H::From { + | ^^^^ diff --git a/ffi/tests/invalid-handle-code/mut-clone-handle.rs b/ffi/tests/invalid-handle-code/mut-clone-handle.rs new file mode 100644 index 000000000..a0eebcc52 --- /dev/null +++ b/ffi/tests/invalid-handle-code/mut-clone-handle.rs @@ -0,0 +1,15 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; +use std::sync::Arc; + +pub struct Foo(u32); + +#[handle_descriptor(target=Foo, mutable=true, sized=true)] +pub struct MutFoo; + +fn main() { + let s = Foo(0); + let h: Handle = Arc::new(s).into(); + let r = h.clone_as_arc(); + let h = h.clone_handle(); +} diff --git a/ffi/tests/invalid-handle-code/mut-clone-handle.stderr b/ffi/tests/invalid-handle-code/mut-clone-handle.stderr new file mode 100644 index 000000000..0d71c6ff8 --- /dev/null +++ b/ffi/tests/invalid-handle-code/mut-clone-handle.stderr @@ -0,0 +1,66 @@ +error[E0599]: the method `clone_as_arc` exists for struct `Handle`, but its trait bounds were not satisfied + --> tests/invalid-handle-code/mut-clone-handle.rs:13:15 + | +8 | pub struct MutFoo; + | ----------------- doesn't satisfy 5 bounds +... +13 | let r = h.clone_as_arc(); + | ^^^^^^^^^^^^ method cannot be called on `Handle` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `::Mutable = False` + `>::From = Arc<_>` + `MutFoo: handle::private::SharedHandleOps<_, _>` + `MutFoo: handle::private::SharedHandle` + which is required by `MutFoo: handle::private::SharedHandleOps<_, _>` + `MutFoo: handle::private::HandleOps<_, False, _>` + which is required by `MutFoo: handle::private::SharedHandleOps<_, _>` +note: the traits `handle::private::SharedHandle`, `handle::private::HandleOps`, and `handle::private::SharedHandleOps` must be implemented + --> src/handle.rs + | + | pub trait SharedHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait HandleOps { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait SharedHandleOps: HandleOps + SharedHandle { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0271]: type mismatch resolving `::Mutable == False` + --> tests/invalid-handle-code/mut-clone-handle.rs:12:41 + | +12 | let h: Handle = Arc::new(s).into(); + | ^^^^ expected `False`, found `True` + | + = note: required for `Handle` to implement `From>` + = note: required for `Arc` to implement `Into>` + +error[E0599]: the method `clone_handle` exists for struct `Handle`, but its trait bounds were not satisfied + --> tests/invalid-handle-code/mut-clone-handle.rs:14:15 + | +8 | pub struct MutFoo; + | ----------------- doesn't satisfy 5 bounds +... +14 | let h = h.clone_handle(); + | ^^^^^^^^^^^^ + | + = note: the following trait bounds were not satisfied: + `::Mutable = False` + `>::From = Arc<_>` + `MutFoo: handle::private::SharedHandleOps<_, _>` + `MutFoo: handle::private::SharedHandle` + which is required by `MutFoo: handle::private::SharedHandleOps<_, _>` + `MutFoo: handle::private::HandleOps<_, False, _>` + which is required by `MutFoo: handle::private::SharedHandleOps<_, _>` +note: the traits `handle::private::SharedHandle`, `handle::private::HandleOps`, and `handle::private::SharedHandleOps` must be implemented + --> src/handle.rs + | + | pub trait SharedHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait HandleOps { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait SharedHandleOps: HandleOps + SharedHandle { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/ffi/tests/invalid-handle-code/not-send.rs b/ffi/tests/invalid-handle-code/not-send.rs new file mode 100644 index 000000000..c6bf998ab --- /dev/null +++ b/ffi/tests/invalid-handle-code/not-send.rs @@ -0,0 +1,11 @@ +use delta_kernel_ffi_macros::handle_descriptor; + +pub struct NotSend(*mut u32); + +#[handle_descriptor(target=NotSend, mutable=false, sized=true)] +pub struct SharedNotSend; + +#[handle_descriptor(target=NotSend, mutable=true, sized=true)] +pub struct MutNotSend; + +fn main() {} diff --git a/ffi/tests/invalid-handle-code/not-send.stderr b/ffi/tests/invalid-handle-code/not-send.stderr new file mode 100644 index 000000000..4f8edfc2b --- /dev/null +++ b/ffi/tests/invalid-handle-code/not-send.stderr @@ -0,0 +1,35 @@ +error[E0277]: `*mut u32` cannot be sent between threads safely + --> tests/invalid-handle-code/not-send.rs:5:28 + | +5 | #[handle_descriptor(target=NotSend, mutable=false, sized=true)] + | ^^^^^^^ `*mut u32` cannot be sent between threads safely + | + = help: within `NotSend`, the trait `Send` is not implemented for `*mut u32`, which is required by `NotSend: Send` +note: required because it appears within the type `NotSend` + --> tests/invalid-handle-code/not-send.rs:3:12 + | +3 | pub struct NotSend(*mut u32); + | ^^^^^^^ +note: required by a bound in `delta_kernel_ffi::handle::HandleDescriptor::Target` + --> src/handle.rs + | + | type Target: ?Sized + Send; + | ^^^^ required by this bound in `HandleDescriptor::Target` + +error[E0277]: `*mut u32` cannot be sent between threads safely + --> tests/invalid-handle-code/not-send.rs:8:28 + | +8 | #[handle_descriptor(target=NotSend, mutable=true, sized=true)] + | ^^^^^^^ `*mut u32` cannot be sent between threads safely + | + = help: within `NotSend`, the trait `Send` is not implemented for `*mut u32`, which is required by `NotSend: Send` +note: required because it appears within the type `NotSend` + --> tests/invalid-handle-code/not-send.rs:3:12 + | +3 | pub struct NotSend(*mut u32); + | ^^^^^^^ +note: required by a bound in `delta_kernel_ffi::handle::HandleDescriptor::Target` + --> src/handle.rs + | + | type Target: ?Sized + Send; + | ^^^^ required by this bound in `HandleDescriptor::Target` diff --git a/ffi/tests/invalid-handle-code/private-constructor.rs b/ffi/tests/invalid-handle-code/private-constructor.rs new file mode 100644 index 000000000..1f67a3331 --- /dev/null +++ b/ffi/tests/invalid-handle-code/private-constructor.rs @@ -0,0 +1,11 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; + +pub struct Foo(u32); + +#[handle_descriptor(target=Foo, mutable=false, sized=true)] +pub struct SharedFoo; + +fn main() { + let _: Handle = Handle { ptr: std::ptr::NonNull::dangling() }; +} diff --git a/ffi/tests/invalid-handle-code/private-constructor.stderr b/ffi/tests/invalid-handle-code/private-constructor.stderr new file mode 100644 index 000000000..b6d9c5e07 --- /dev/null +++ b/ffi/tests/invalid-handle-code/private-constructor.stderr @@ -0,0 +1,5 @@ +error[E0451]: field `ptr` of struct `Handle` is private + --> tests/invalid-handle-code/private-constructor.rs:10:41 + | +10 | let _: Handle = Handle { ptr: std::ptr::NonNull::dangling() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ private field diff --git a/ffi/tests/invalid-handle-code/shared-as-mut.rs b/ffi/tests/invalid-handle-code/shared-as-mut.rs new file mode 100644 index 000000000..89d25c8d1 --- /dev/null +++ b/ffi/tests/invalid-handle-code/shared-as-mut.rs @@ -0,0 +1,13 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; + +pub struct Foo(u32); + +#[handle_descriptor(target=Foo, mutable=false, sized=true)] +pub struct SharedFoo; + +fn main() { + let s = Foo(0); + let h: Handle = Box::new(s).into(); + let r = h.as_mut(); +} diff --git a/ffi/tests/invalid-handle-code/shared-as-mut.stderr b/ffi/tests/invalid-handle-code/shared-as-mut.stderr new file mode 100644 index 000000000..d97132513 --- /dev/null +++ b/ffi/tests/invalid-handle-code/shared-as-mut.stderr @@ -0,0 +1,36 @@ +error[E0599]: the method `as_mut` exists for struct `Handle`, but its trait bounds were not satisfied + --> tests/invalid-handle-code/shared-as-mut.rs:12:15 + | +7 | pub struct SharedFoo; + | -------------------- doesn't satisfy `::Mutable = True`, `SharedFoo: handle::private::HandleOps<_, True, _>`, `SharedFoo: handle::private::MutableHandleOps<_, _>` or `SharedFoo: handle::private::MutableHandle` +... +12 | let r = h.as_mut(); + | ^^^^^^ method cannot be called on `Handle` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `::Mutable = True` + `SharedFoo: handle::private::MutableHandleOps<_, _>` + `SharedFoo: handle::private::MutableHandle` + which is required by `SharedFoo: handle::private::MutableHandleOps<_, _>` + `SharedFoo: handle::private::HandleOps<_, True, _>` + which is required by `SharedFoo: handle::private::MutableHandleOps<_, _>` +note: the traits `handle::private::MutableHandle`, `handle::private::HandleOps`, and `handle::private::MutableHandleOps` must be implemented + --> src/handle.rs + | + | pub trait MutableHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait HandleOps { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait MutableHandleOps: HandleOps + MutableHandle { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0271]: type mismatch resolving `::Mutable == True` + --> tests/invalid-handle-code/shared-as-mut.rs:11:44 + | +11 | let h: Handle = Box::new(s).into(); + | ^^^^ expected `True`, found `False` + | + = note: required for `Handle` to implement `From>` + = note: required for `Box` to implement `Into>` diff --git a/ffi/tests/invalid-handle-code/shared-not-sync.rs b/ffi/tests/invalid-handle-code/shared-not-sync.rs new file mode 100644 index 000000000..91a3e9816 --- /dev/null +++ b/ffi/tests/invalid-handle-code/shared-not-sync.rs @@ -0,0 +1,17 @@ +use delta_kernel_ffi_macros::handle_descriptor; +use delta_kernel_ffi::handle::Handle; +use std::sync::Arc; + +pub struct NotSync(*mut u32); + +unsafe impl Send for NotSync {} + +#[handle_descriptor(target=NotSync, mutable=false, sized=true)] +pub struct SharedNotSync; + +fn main() { + let s = NotSync(std::ptr::null_mut()); + let h: Handle = Arc::new(s).into(); + let r = h.clone_as_arc(); + let h = h.clone_handle(); +} diff --git a/ffi/tests/invalid-handle-code/shared-not-sync.stderr b/ffi/tests/invalid-handle-code/shared-not-sync.stderr new file mode 100644 index 000000000..805f4e62b --- /dev/null +++ b/ffi/tests/invalid-handle-code/shared-not-sync.stderr @@ -0,0 +1,59 @@ +error[E0599]: the method `clone_as_arc` exists for struct `Handle`, but its trait bounds were not satisfied + --> tests/invalid-handle-code/shared-not-sync.rs:15:15 + | +10 | pub struct SharedNotSync; + | ------------------------ doesn't satisfy `SharedNotSync: handle::private::SharedHandle` or `_: SharedHandleOps<_, _>` +... +15 | let r = h.clone_as_arc(); + | ^^^^^^^^^^^^ method cannot be called on `Handle` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `SharedNotSync: handle::private::SharedHandleOps<_, _>` + `SharedNotSync: handle::private::SharedHandle` + which is required by `SharedNotSync: handle::private::SharedHandleOps<_, _>` +note: the traits `handle::private::SharedHandle` and `handle::private::SharedHandleOps` must be implemented + --> src/handle.rs + | + | pub trait SharedHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait SharedHandleOps: HandleOps + SharedHandle { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0277]: `*mut u32` cannot be shared between threads safely + --> tests/invalid-handle-code/shared-not-sync.rs:14:48 + | +14 | let h: Handle = Arc::new(s).into(); + | ^^^^ `*mut u32` cannot be shared between threads safely + | + = help: within `NotSync`, the trait `Sync` is not implemented for `*mut u32`, which is required by `Arc: Into<_>` +note: required because it appears within the type `NotSync` + --> tests/invalid-handle-code/shared-not-sync.rs:5:12 + | +5 | pub struct NotSync(*mut u32); + | ^^^^^^^ + = note: required for `SharedNotSync` to implement `handle::private::SharedHandleOps` + = note: required for `Handle` to implement `From>` + = note: required for `Arc` to implement `Into>` + +error[E0599]: the method `clone_handle` exists for struct `Handle`, but its trait bounds were not satisfied + --> tests/invalid-handle-code/shared-not-sync.rs:16:15 + | +10 | pub struct SharedNotSync; + | ------------------------ doesn't satisfy `SharedNotSync: handle::private::SharedHandle` or `_: SharedHandleOps<_, _>` +... +16 | let h = h.clone_handle(); + | ^^^^^^^^^^^^ + | + = note: the following trait bounds were not satisfied: + `SharedNotSync: handle::private::SharedHandleOps<_, _>` + `SharedNotSync: handle::private::SharedHandle` + which is required by `SharedNotSync: handle::private::SharedHandleOps<_, _>` +note: the traits `handle::private::SharedHandle` and `handle::private::SharedHandleOps` must be implemented + --> src/handle.rs + | + | pub trait SharedHandle {} + | ^^^^^^^^^^^^^^^^^^^^^^ +... + | pub trait SharedHandleOps: HandleOps + SharedHandle { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/kernel/build.rs b/kernel/build.rs index b1d81b235..5ef8246e7 100644 --- a/kernel/build.rs +++ b/kernel/build.rs @@ -3,6 +3,7 @@ use rustc_version::{version_meta, Channel}; fn main() { // note if we're on the nightly channel so we can enable doc_auto_cfg if so if let Channel::Nightly = version_meta().unwrap().channel { + println!("cargo::rustc-check-cfg=cfg(NIGHTLY_CHANNEL)"); println!("cargo:rustc-cfg=NIGHTLY_CHANNEL"); } } diff --git a/kernel/src/actions/visitors.rs b/kernel/src/actions/visitors.rs index ff32146d7..258583015 100644 --- a/kernel/src/actions/visitors.rs +++ b/kernel/src/actions/visitors.rs @@ -1,5 +1,5 @@ //! This module defines visitors that can be used to extract the various delta actions from -//! [`EngineData`] types. +//! [`crate::engine_data::EngineData`] types. use std::collections::HashMap; diff --git a/kernel/src/engine/arrow_utils.rs b/kernel/src/engine/arrow_utils.rs index 081ae1756..1e15199e4 100644 --- a/kernel/src/engine/arrow_utils.rs +++ b/kernel/src/engine/arrow_utils.rs @@ -62,10 +62,8 @@ pub(crate) fn generate_mask( } } -/// Reorder a RecordBatch to match `requested_ordering`. This method takes `mask_indicies` as -/// computed by [`get_requested_indicies`] as an optimization. If the indicies are in order, then we -/// don't need to do any re-ordering. Otherwise, for each non-zero value in `requested_ordering`, -/// the column at that index will be added in order to returned batch +/// Reorder a RecordBatch to match `requested_ordering`. For each non-zero value in +/// `requested_ordering`, the column at that index will be added in order to returned batch pub(crate) fn reorder_record_batch( input_data: RecordBatch, requested_ordering: &[usize], diff --git a/kernel/src/scan/data_skipping.rs b/kernel/src/scan/data_skipping.rs index b6ffc351b..d7b06f099 100644 --- a/kernel/src/scan/data_skipping.rs +++ b/kernel/src/scan/data_skipping.rs @@ -9,7 +9,7 @@ use crate::expressions::{BinaryOperator, Expression as Expr, VariadicOperator}; use crate::schema::{DataType, SchemaRef, StructField, StructType}; use crate::{Engine, EngineData, ExpressionEvaluator, JsonHandler}; -/// Returns (if any) such that B A is equivalent to A B. +/// Returns `` (if any) such that `B A` is equivalent to `A B`. fn commute(op: &BinaryOperator) -> Option { use BinaryOperator::*; match op {