Skip to content

Commit

Permalink
Rollup merge of #106675 - krtab:fix_improper_ctypes, r=davidtwco
Browse files Browse the repository at this point in the history
Mark ZST as FFI-safe if all its fields are PhantomData

This presents one possible solution to issue: #106629.

This is my first (tentative) contribution to the compiler itself.

I'm looking forward for comments and feedback

Closes: #106629
  • Loading branch information
Yuki Okushi authored Jan 12, 2023
2 parents 3f21b81 + 797f247 commit 19ba430
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 23 deletions.
46 changes: 23 additions & 23 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,39 +878,39 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
) -> FfiResult<'tcx> {
use FfiResult::*;

if def.repr().transparent() {
let transparent_safety = def.repr().transparent().then(|| {
// Can assume that at most one field is not a ZST, so only check
// that field's type for FFI-safety.
if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) {
self.check_field_type_for_ffi(cache, field, substs)
return self.check_field_type_for_ffi(cache, field, substs);
} else {
// All fields are ZSTs; this means that the type should behave
// like (), which is FFI-unsafe
// like (), which is FFI-unsafe... except if all fields are PhantomData,
// which is tested for below
FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None }
}
} else {
// We can't completely trust repr(C) markings; make sure the fields are
// actually safe.
let mut all_phantom = !variant.fields.is_empty();
for field in &variant.fields {
match self.check_field_type_for_ffi(cache, &field, substs) {
FfiSafe => {
all_phantom = false;
}
FfiPhantom(..) if def.is_enum() => {
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_enum_phantomdata,
help: None,
};
}
FfiPhantom(..) => {}
r => return r,
});
// We can't completely trust repr(C) markings; make sure the fields are
// actually safe.
let mut all_phantom = !variant.fields.is_empty();
for field in &variant.fields {
match self.check_field_type_for_ffi(cache, &field, substs) {
FfiSafe => {
all_phantom = false;
}
FfiPhantom(..) if !def.repr().transparent() && def.is_enum() => {
return FfiUnsafe {
ty,
reason: fluent::lint_improper_ctypes_enum_phantomdata,
help: None,
};
}
FfiPhantom(..) => {}
r => return transparent_safety.unwrap_or(r),
}

if all_phantom { FfiPhantom(ty) } else { FfiSafe }
}

if all_phantom { FfiPhantom(ty) } else { transparent_safety.unwrap_or(FfiSafe) }
}

/// Checks if the given type is "ffi-safe" (has a stable, well-defined
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/lint/lint-ffi-safety-all-phantom.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// This is a regression test for issue https://github.com/rust-lang/rust/issues/106629.
// It ensures that transparent types where all fields are PhantomData are marked as
// FFI-safe.

// check-pass

#[repr(transparent)]
#[derive(Copy, Clone)]
struct MyPhantom(core::marker::PhantomData<u8>);

#[repr(C)]
#[derive(Copy, Clone)]
pub struct Bar {
pub x: i32,
_marker: MyPhantom,
}

extern "C" {
pub fn foo(bar: *mut Bar);
}

fn main() {}

0 comments on commit 19ba430

Please sign in to comment.