diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index b60867b3d2d45..cdb0eda645a48 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -6,7 +6,6 @@ use rustc_attr as attr; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::def_id::DefId; use rustc_hir::{is_range_literal, ExprKind, Node}; use rustc_index::vec::Idx; use rustc_middle::mir::interpret::{sign_extend, truncate}; @@ -511,10 +510,6 @@ enum FfiResult<'tcx> { FfiUnsafe { ty: Ty<'tcx>, reason: &'static str, help: Option<&'static str> }, } -fn is_zst<'tcx>(tcx: TyCtxt<'tcx>, did: DefId, ty: Ty<'tcx>) -> bool { - tcx.layout_of(tcx.param_env(did).and(ty)).map(|layout| layout.is_zst()).unwrap_or(false) -} - fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { match ty.kind { ty::FnPtr(_) => true, @@ -523,7 +518,7 @@ fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { for field in field_def.all_fields() { let field_ty = tcx.normalize_erasing_regions(ParamEnv::reveal_all(), field.ty(tcx, substs)); - if is_zst(tcx, field.did, field_ty) { + if field_ty.is_zst(tcx, field.did) { continue; } @@ -653,32 +648,43 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } - // We can't completely trust repr(C) and repr(transparent) markings; - // make sure the fields are actually safe. - let mut all_phantom = true; - for field in &def.non_enum_variant().fields { - let field_ty = cx.normalize_erasing_regions( - ParamEnv::reveal_all(), - field.ty(cx, substs), - ); - // repr(transparent) types are allowed to have arbitrary ZSTs, not just - // PhantomData -- skip checking all ZST fields - if def.repr.transparent() && is_zst(cx, field.did, field_ty) { - continue; + if def.repr.transparent() { + // Can assume that only one field is not a ZST, so only check + // that field's type for FFI-safety. + if let Some(field) = + def.transparent_newtype_field(cx, self.cx.param_env) + { + let field_ty = cx.normalize_erasing_regions( + self.cx.param_env, + field.ty(cx, substs), + ); + self.check_type_for_ffi(cache, field_ty) + } else { + FfiSafe } - let r = self.check_type_for_ffi(cache, field_ty); - match r { - FfiSafe => { - all_phantom = false; - } - FfiPhantom(..) => {} - FfiUnsafe { .. } => { - return r; + } else { + // We can't completely trust repr(C) markings; make sure the fields are + // actually safe. + let mut all_phantom = true; + for field in &def.non_enum_variant().fields { + let field_ty = cx.normalize_erasing_regions( + self.cx.param_env, + field.ty(cx, substs), + ); + let r = self.check_type_for_ffi(cache, field_ty); + match r { + FfiSafe => { + all_phantom = false; + } + FfiPhantom(..) => {} + FfiUnsafe { .. } => { + return r; + } } } - } - if all_phantom { FfiPhantom(ty) } else { FfiSafe } + if all_phantom { FfiPhantom(ty) } else { FfiSafe } + } } AdtKind::Union => { if !def.repr.c() && !def.repr.transparent() { @@ -708,7 +714,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ); // repr(transparent) types are allowed to have arbitrary ZSTs, not just // PhantomData -- skip checking all ZST fields. - if def.repr.transparent() && is_zst(cx, field.did, field_ty) { + if def.repr.transparent() && field_ty.is_zst(cx, field.did) { continue; } let r = self.check_type_for_ffi(cache, field_ty); @@ -774,7 +780,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ); // repr(transparent) types are allowed to have arbitrary ZSTs, not // just PhantomData -- skip checking all ZST fields. - if def.repr.transparent() && is_zst(cx, field.did, field_ty) { + if def.repr.transparent() && field_ty.is_zst(cx, field.did) { continue; } let r = self.check_type_for_ffi(cache, field_ty); @@ -983,6 +989,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiResult::FfiPhantom(ty) => { self.emit_ffi_unsafe_type_lint(ty, sp, "composed only of `PhantomData`", None); } + // If `ty` is a `repr(transparent)` newtype, and the non-zero-sized type is a generic + // argument, which after substitution, is `()`, then this branch can be hit. + FfiResult::FfiUnsafe { ty, .. } if is_return_type && ty.is_unit() => return, FfiResult::FfiUnsafe { ty, reason, help } => { self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); } diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index ffbe3a40297c1..caa1b4cb375fe 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -2390,6 +2390,29 @@ impl<'tcx> AdtDef { pub fn sized_constraint(&self, tcx: TyCtxt<'tcx>) -> &'tcx [Ty<'tcx>] { tcx.adt_sized_constraint(self.did).0 } + + /// `repr(transparent)` structs can have a single non-ZST field, this function returns that + /// field. + pub fn transparent_newtype_field( + &self, + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + ) -> Option<&FieldDef> { + assert!(self.is_struct() && self.repr.transparent()); + + for field in &self.non_enum_variant().fields { + let field_ty = tcx.normalize_erasing_regions( + param_env, + field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.did)), + ); + + if !field_ty.is_zst(tcx, self.did) { + return Some(field); + } + } + + None + } } impl<'tcx> FieldDef { diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index 5d4c2a54267c3..7550be39d4ab0 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -2186,6 +2186,11 @@ impl<'tcx> TyS<'tcx> { } } } + + /// Is this a zero-sized type? + pub fn is_zst(&'tcx self, tcx: TyCtxt<'tcx>, did: DefId) -> bool { + tcx.layout_of(tcx.param_env(did).and(self)).map(|layout| layout.is_zst()).unwrap_or(false) + } } /// Typed constant value. diff --git a/src/test/ui/lint/lint-ctypes-66202.rs b/src/test/ui/lint/lint-ctypes-66202.rs index 3fe4560f44bcc..ebab41d143e67 100644 --- a/src/test/ui/lint/lint-ctypes-66202.rs +++ b/src/test/ui/lint/lint-ctypes-66202.rs @@ -1,3 +1,5 @@ +// check-pass + #![deny(improper_ctypes)] // This test checks that return types are normalized before being checked for FFI-safety, and that @@ -10,7 +12,6 @@ extern "C" { pub fn bare() -> (); pub fn normalize() -> <() as ToOwned>::Owned; pub fn transparent() -> W<()>; - //~^ ERROR uses type `W<()>` } fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-66202.stderr b/src/test/ui/lint/lint-ctypes-66202.stderr deleted file mode 100644 index 759c77deadc7b..0000000000000 --- a/src/test/ui/lint/lint-ctypes-66202.stderr +++ /dev/null @@ -1,20 +0,0 @@ -error: `extern` block uses type `W<()>`, which is not FFI-safe - --> $DIR/lint-ctypes-66202.rs:12:29 - | -LL | pub fn transparent() -> W<()>; - | ^^^^^ not FFI-safe - | -note: the lint level is defined here - --> $DIR/lint-ctypes-66202.rs:1:9 - | -LL | #![deny(improper_ctypes)] - | ^^^^^^^^^^^^^^^ - = note: composed only of `PhantomData` -note: the type is defined here - --> $DIR/lint-ctypes-66202.rs:7:1 - | -LL | pub struct W(T); - | ^^^^^^^^^^^^^^^^^^^ - -error: aborting due to previous error -