From d5b07373cef98dec5672cd94bf14609eec65aa03 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 11 Jun 2020 22:53:32 +0100 Subject: [PATCH 1/4] ty: projections in `transparent_newtype_field` This commit modifies `transparent_newtype_field` so that it handles projections with generic parameters, where `normalize_erasing_regions` would ICE. Signed-off-by: David Wood --- src/librustc_middle/ty/mod.rs | 12 ++++++++---- src/test/ui/lint/lint-ctypes-73249-1.rs | 21 +++++++++++++++++++++ src/test/ui/lint/lint-ctypes-73249.rs | 21 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-73249-1.rs create mode 100644 src/test/ui/lint/lint-ctypes-73249.rs diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 93ef73171993c..1f94c28b35723 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -2387,11 +2387,15 @@ impl<'tcx> AdtDef { 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)), - ); + let field_ty = field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.did)); + + // `normalize_erasing_regions` will fail for projections that contain generic + // parameters, so check these before normalizing. + if field_ty.has_projections() && field_ty.needs_subst() { + return Some(field); + } + let field_ty = tcx.normalize_erasing_regions(param_env, field_ty); if !field_ty.is_zst(tcx, self.did) { return Some(field); } diff --git a/src/test/ui/lint/lint-ctypes-73249-1.rs b/src/test/ui/lint/lint-ctypes-73249-1.rs new file mode 100644 index 0000000000000..cf416c3fe8b12 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-1.rs @@ -0,0 +1,21 @@ +// check-pass +#![deny(improper_ctypes)] + +pub trait Foo { + type Assoc: 'static; +} + +impl Foo for () { + type Assoc = u32; +} + +extern "C" { + pub fn lint_me(x: Bar<()>); +} + +#[repr(transparent)] +pub struct Bar { + value: &'static ::Assoc, +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73249.rs b/src/test/ui/lint/lint-ctypes-73249.rs new file mode 100644 index 0000000000000..5b48fa9b7376f --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249.rs @@ -0,0 +1,21 @@ +// check-pass +#![deny(improper_ctypes)] + +pub trait Foo { + type Assoc; +} + +impl Foo for () { + type Assoc = u32; +} + +extern "C" { + pub fn lint_me(x: Bar<()>); +} + +#[repr(transparent)] +pub struct Bar { + value: ::Assoc, +} + +fn main() {} From 76ad38d992835292f60f5f09b04b2b0886e312d9 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Jun 2020 13:30:39 +0100 Subject: [PATCH 2/4] lint: prohibit fields with opaque types Opaque types cannot be used in extern declarations, and normally cannot exist in fields - except with type aliases to `impl Trait` and projections which normalize to them. Signed-off-by: David Wood --- src/librustc_lint/types.rs | 39 +++++++++++++-------- src/test/ui/lint/lint-ctypes-73249-2.rs | 29 +++++++++++++++ src/test/ui/lint/lint-ctypes-73249-2.stderr | 15 ++++++++ src/test/ui/lint/lint-ctypes-73249-3.rs | 21 +++++++++++ src/test/ui/lint/lint-ctypes-73249-3.stderr | 15 ++++++++ src/test/ui/lint/lint-ctypes-73249-5.rs | 21 +++++++++++ src/test/ui/lint/lint-ctypes-73249-5.stderr | 15 ++++++++ 7 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-73249-2.rs create mode 100644 src/test/ui/lint/lint-ctypes-73249-2.stderr create mode 100644 src/test/ui/lint/lint-ctypes-73249-3.rs create mode 100644 src/test/ui/lint/lint-ctypes-73249-3.stderr create mode 100644 src/test/ui/lint/lint-ctypes-73249-5.rs create mode 100644 src/test/ui/lint/lint-ctypes-73249-5.stderr diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 1dd6d837d4eaf..f429eef90c8c8 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -11,7 +11,7 @@ use rustc_index::vec::Idx; use rustc_middle::mir::interpret::{sign_extend, truncate}; use rustc_middle::ty::layout::{IntegerExt, SizeSkeleton}; use rustc_middle::ty::subst::SubstsRef; -use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; +use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt, TypeFoldable}; use rustc_span::source_map; use rustc_span::symbol::sym; use rustc_span::Span; @@ -597,6 +597,22 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } + /// Checks if the given field's type is "ffi-safe". + fn check_field_type_for_ffi( + &self, + cache: &mut FxHashSet>, + field: &ty::FieldDef, + substs: SubstsRef<'tcx>, + ) -> FfiResult<'tcx> { + let field_ty = field.ty(self.cx.tcx, substs); + if field_ty.has_opaque_types() { + self.check_type_for_ffi(cache, field_ty) + } else { + let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty); + self.check_type_for_ffi(cache, field_ty) + } + } + /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> FfiResult<'tcx> { @@ -654,11 +670,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { 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) + self.check_field_type_for_ffi(cache, field, substs) } else { FfiSafe } @@ -667,11 +679,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // 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); + let r = self.check_field_type_for_ffi(cache, field, substs); match r { FfiSafe => { all_phantom = false; @@ -886,6 +894,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Foreign(..) => FfiSafe, + // While opaque types are checked for earlier, if a projection in a struct field + // normalizes to an opaque type, then it will reach this branch. + ty::Opaque(..) => { + FfiUnsafe { ty, reason: "opaque types have no C equivalent", help: None } + } + ty::Param(..) | ty::Infer(..) | ty::Bound(..) @@ -895,7 +909,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { | ty::GeneratorWitness(..) | ty::Placeholder(..) | ty::Projection(..) - | ty::Opaque(..) | ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty), } } @@ -925,8 +938,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { - use rustc_middle::ty::TypeFoldable; - struct ProhibitOpaqueTypes<'tcx> { ty: Option>, }; diff --git a/src/test/ui/lint/lint-ctypes-73249-2.rs b/src/test/ui/lint/lint-ctypes-73249-2.rs new file mode 100644 index 0000000000000..86cc5e2c31e81 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-2.rs @@ -0,0 +1,29 @@ +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait Baz { } + +impl Baz for () { } + +type Qux = impl Baz; + +fn assign() -> Qux {} + +pub trait Foo { + type Assoc: 'static; +} + +impl Foo for () { + type Assoc = Qux; +} + +#[repr(transparent)] +pub struct A { + x: &'static ::Assoc, +} + +extern "C" { + pub fn lint_me() -> A<()>; //~ ERROR: uses type `impl Baz` +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73249-2.stderr b/src/test/ui/lint/lint-ctypes-73249-2.stderr new file mode 100644 index 0000000000000..36dbe3217d75a --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-2.stderr @@ -0,0 +1,15 @@ +error: `extern` block uses type `impl Baz`, which is not FFI-safe + --> $DIR/lint-ctypes-73249-2.rs:26:25 + | +LL | pub fn lint_me() -> A<()>; + | ^^^^^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-73249-2.rs:2:9 + | +LL | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + = note: opaque types have no C equivalent + +error: aborting due to previous error + diff --git a/src/test/ui/lint/lint-ctypes-73249-3.rs b/src/test/ui/lint/lint-ctypes-73249-3.rs new file mode 100644 index 0000000000000..25c4e7c92a854 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-3.rs @@ -0,0 +1,21 @@ +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait Baz { } + +impl Baz for u32 { } + +type Qux = impl Baz; + +fn assign() -> Qux { 3 } + +#[repr(C)] +pub struct A { + x: Qux, +} + +extern "C" { + pub fn lint_me() -> A; //~ ERROR: uses type `impl Baz` +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73249-3.stderr b/src/test/ui/lint/lint-ctypes-73249-3.stderr new file mode 100644 index 0000000000000..7d133287bd73e --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-3.stderr @@ -0,0 +1,15 @@ +error: `extern` block uses type `impl Baz`, which is not FFI-safe + --> $DIR/lint-ctypes-73249-3.rs:18:25 + | +LL | pub fn lint_me() -> A; + | ^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-73249-3.rs:2:9 + | +LL | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + = note: opaque types have no C equivalent + +error: aborting due to previous error + diff --git a/src/test/ui/lint/lint-ctypes-73249-5.rs b/src/test/ui/lint/lint-ctypes-73249-5.rs new file mode 100644 index 0000000000000..61e46983ede65 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-5.rs @@ -0,0 +1,21 @@ +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait Baz { } + +impl Baz for u32 { } + +type Qux = impl Baz; + +fn assign() -> Qux { 3 } + +#[repr(transparent)] +pub struct A { + x: Qux, +} + +extern "C" { + pub fn lint_me() -> A; //~ ERROR: uses type `impl Baz` +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73249-5.stderr b/src/test/ui/lint/lint-ctypes-73249-5.stderr new file mode 100644 index 0000000000000..d2780cb60e7dd --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-5.stderr @@ -0,0 +1,15 @@ +error: `extern` block uses type `impl Baz`, which is not FFI-safe + --> $DIR/lint-ctypes-73249-5.rs:18:25 + | +LL | pub fn lint_me() -> A; + | ^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-73249-5.rs:2:9 + | +LL | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + = note: opaque types have no C equivalent + +error: aborting due to previous error + From 0cccaa0a27499f742ed4dbf4b5d09beee08eb2d6 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Jun 2020 14:11:45 +0100 Subject: [PATCH 3/4] lint: unify enum variant, union and struct logic This commit applies the changes introduced in #72890 to both enum variants and unions - where the logic prior to #72890 was duplicated. Signed-off-by: David Wood --- src/librustc_lint/types.rs | 207 ++++++++++++++-------------------- src/librustc_middle/ty/mod.rs | 52 ++++----- 2 files changed, 107 insertions(+), 152 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index f429eef90c8c8..b20bf43261474 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -507,7 +507,7 @@ struct ImproperCTypesVisitor<'a, 'tcx> { enum FfiResult<'tcx> { FfiSafe, FfiPhantom(Ty<'tcx>), - FfiUnsafe { ty: Ty<'tcx>, reason: &'static str, help: Option<&'static str> }, + FfiUnsafe { ty: Ty<'tcx>, reason: String, help: Option }, } fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { @@ -613,6 +613,50 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } + /// Checks if the given `VariantDef`'s field types are "ffi-safe". + fn check_variant_for_ffi( + &self, + cache: &mut FxHashSet>, + ty: Ty<'tcx>, + def: &ty::AdtDef, + variant: &ty::VariantDef, + substs: SubstsRef<'tcx>, + ) -> FfiResult<'tcx> { + use FfiResult::*; + + 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) = variant.transparent_newtype_field(self.cx.tcx, self.cx.param_env) { + self.check_field_type_for_ffi(cache, field, substs) + } else { + bug!("malformed transparent type"); + } + } 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: "this enum contains a PhantomData field".into(), + help: None, + }; + } + FfiPhantom(..) => {} + r => return r, + } + } + + if all_phantom { FfiPhantom(ty) } else { FfiSafe } + } + } + /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> FfiResult<'tcx> { @@ -634,15 +678,18 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return FfiPhantom(ty); } match def.adt_kind() { - AdtKind::Struct => { + AdtKind::Struct | AdtKind::Union => { + let kind = if def.is_struct() { "struct" } else { "union" }; + if !def.repr.c() && !def.repr.transparent() { return FfiUnsafe { ty, - reason: "this struct has unspecified layout", - help: Some( + reason: format!("this {} has unspecified layout", kind), + help: Some(format!( "consider adding a `#[repr(C)]` or \ - `#[repr(transparent)]` attribute to this struct", - ), + `#[repr(transparent)]` attribute to this {}", + kind + )), }; } @@ -651,7 +698,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if is_non_exhaustive && !def.did.is_local() { return FfiUnsafe { ty, - reason: "this struct is non-exhaustive", + reason: format!("this {} is non-exhaustive", kind), help: None, }; } @@ -659,85 +706,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if def.non_enum_variant().fields.is_empty() { return FfiUnsafe { ty, - reason: "this struct has no fields", - help: Some("consider adding a member to this struct"), + reason: format!("this {} has no fields", kind), + help: Some(format!("consider adding a member to this {}", kind)), }; } - 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) - { - self.check_field_type_for_ffi(cache, field, substs) - } else { - FfiSafe - } - } 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 r = self.check_field_type_for_ffi(cache, field, substs); - match r { - FfiSafe => { - all_phantom = false; - } - FfiPhantom(..) => {} - FfiUnsafe { .. } => { - return r; - } - } - } - - if all_phantom { FfiPhantom(ty) } else { FfiSafe } - } - } - AdtKind::Union => { - if !def.repr.c() && !def.repr.transparent() { - return FfiUnsafe { - ty, - reason: "this union has unspecified layout", - help: Some( - "consider adding a `#[repr(C)]` or \ - `#[repr(transparent)]` attribute to this union", - ), - }; - } - - if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe { - ty, - reason: "this union has no fields", - help: Some("consider adding a field to this union"), - }; - } - - 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() && field_ty.is_zst(cx, field.did) { - continue; - } - 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 } + self.check_variant_for_ffi(cache, ty, def, def.non_enum_variant(), substs) } AdtKind::Enum => { if def.variants.is_empty() { @@ -752,11 +726,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !is_repr_nullable_ptr(cx, ty, def, substs) { return FfiUnsafe { ty, - reason: "enum has no representation hint", + reason: "enum has no representation hint".into(), help: Some( "consider adding a `#[repr(C)]`, \ `#[repr(transparent)]`, or integer `#[repr(...)]` \ - attribute to this enum", + attribute to this enum" + .into(), ), }; } @@ -765,7 +740,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if def.is_variant_list_non_exhaustive() && !def.did.is_local() { return FfiUnsafe { ty, - reason: "this enum is non-exhaustive", + reason: "this enum is non-exhaustive".into(), help: None, }; } @@ -776,37 +751,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if is_non_exhaustive && !variant.def_id.is_local() { return FfiUnsafe { ty, - reason: "this enum has non-exhaustive variants", + reason: "this enum has non-exhaustive variants".into(), help: None, }; } - for field in &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() && field_ty.is_zst(cx, field.did) { - continue; - } - let r = self.check_type_for_ffi(cache, field_ty); - match r { - FfiSafe => {} - FfiUnsafe { .. } => { - return r; - } - FfiPhantom(..) => { - return FfiUnsafe { - ty, - reason: "this enum contains a PhantomData field", - help: None, - }; - } - } + match self.check_variant_for_ffi(cache, ty, def, variant, substs) { + FfiSafe => (), + r => return r, } } + FfiSafe } } @@ -814,13 +769,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Char => FfiUnsafe { ty, - reason: "the `char` type has no C equivalent", - help: Some("consider using `u32` or `libc::wchar_t` instead"), + reason: "the `char` type has no C equivalent".into(), + help: Some("consider using `u32` or `libc::wchar_t` instead".into()), }, ty::Int(ast::IntTy::I128) | ty::Uint(ast::UintTy::U128) => FfiUnsafe { ty, - reason: "128-bit integers don't currently have a known stable ABI", + reason: "128-bit integers don't currently have a known stable ABI".into(), help: None, }, @@ -829,24 +784,24 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Slice(_) => FfiUnsafe { ty, - reason: "slices have no C equivalent", - help: Some("consider using a raw pointer instead"), + reason: "slices have no C equivalent".into(), + help: Some("consider using a raw pointer instead".into()), }, ty::Dynamic(..) => { - FfiUnsafe { ty, reason: "trait objects have no C equivalent", help: None } + FfiUnsafe { ty, reason: "trait objects have no C equivalent".into(), help: None } } ty::Str => FfiUnsafe { ty, - reason: "string slices have no C equivalent", - help: Some("consider using `*const u8` and a length instead"), + reason: "string slices have no C equivalent".into(), + help: Some("consider using `*const u8` and a length instead".into()), }, ty::Tuple(..) => FfiUnsafe { ty, - reason: "tuples have unspecified layout", - help: Some("consider using a struct instead"), + reason: "tuples have unspecified layout".into(), + help: Some("consider using a struct instead".into()), }, ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) => { @@ -860,10 +815,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic | Abi::RustCall => { return FfiUnsafe { ty, - reason: "this function pointer has Rust-specific calling convention", + reason: "this function pointer has Rust-specific calling convention" + .into(), help: Some( "consider using an `extern fn(...) -> ...` \ - function pointer instead", + function pointer instead" + .into(), ), }; } @@ -897,7 +854,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // While opaque types are checked for earlier, if a projection in a struct field // normalizes to an opaque type, then it will reach this branch. ty::Opaque(..) => { - FfiUnsafe { ty, reason: "opaque types have no C equivalent", help: None } + FfiUnsafe { ty, reason: "opaque types have no C equivalent".into(), help: None } } ty::Param(..) @@ -1004,7 +961,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // 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); + self.emit_ffi_unsafe_type_lint(ty, sp, &reason, help.as_deref()); } } } diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 1f94c28b35723..30643165ce9cc 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -1807,6 +1807,31 @@ impl<'tcx> VariantDef { pub fn is_field_list_non_exhaustive(&self) -> bool { self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE) } + + /// `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> { + for field in &self.fields { + let field_ty = field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.def_id)); + + // `normalize_erasing_regions` will fail for projections that contain generic + // parameters, so check these before normalizing. + if field_ty.has_projections() && field_ty.needs_subst() { + return Some(field); + } + + let field_ty = tcx.normalize_erasing_regions(param_env, field_ty); + if !field_ty.is_zst(tcx, self.def_id) { + return Some(field); + } + } + + None + } } #[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)] @@ -2376,33 +2401,6 @@ 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 = field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.did)); - - // `normalize_erasing_regions` will fail for projections that contain generic - // parameters, so check these before normalizing. - if field_ty.has_projections() && field_ty.needs_subst() { - return Some(field); - } - - let field_ty = tcx.normalize_erasing_regions(param_env, field_ty); - if !field_ty.is_zst(tcx, self.did) { - return Some(field); - } - } - - None - } } impl<'tcx> FieldDef { From a730d888aee99f9a556dae799deadacb83c34d0e Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Jun 2020 16:14:05 +0100 Subject: [PATCH 4/4] ty: simplify `transparent_newtype_field` This commit removes the normalization from `transparent_newtype_field` - turns out it wasn't necessary and that makes it a bunch simpler - particularly when handling projections. Signed-off-by: David Wood --- src/librustc_lint/types.rs | 2 +- src/librustc_middle/ty/mod.rs | 14 +------------- src/test/ui/lint/lint-ctypes-73249-4.rs | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-73249-4.rs diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index b20bf43261474..b490c06403d75 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -627,7 +627,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { 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) = variant.transparent_newtype_field(self.cx.tcx, self.cx.param_env) { + if let Some(field) = variant.transparent_newtype_field(self.cx.tcx) { self.check_field_type_for_ffi(cache, field, substs) } else { bug!("malformed transparent type"); diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 30643165ce9cc..b4e3e6ab5ab7e 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -1810,21 +1810,9 @@ impl<'tcx> VariantDef { /// `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> { + pub fn transparent_newtype_field(&self, tcx: TyCtxt<'tcx>) -> Option<&FieldDef> { for field in &self.fields { let field_ty = field.ty(tcx, InternalSubsts::identity_for_item(tcx, self.def_id)); - - // `normalize_erasing_regions` will fail for projections that contain generic - // parameters, so check these before normalizing. - if field_ty.has_projections() && field_ty.needs_subst() { - return Some(field); - } - - let field_ty = tcx.normalize_erasing_regions(param_env, field_ty); if !field_ty.is_zst(tcx, self.def_id) { return Some(field); } diff --git a/src/test/ui/lint/lint-ctypes-73249-4.rs b/src/test/ui/lint/lint-ctypes-73249-4.rs new file mode 100644 index 0000000000000..6c72bd691b17c --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73249-4.rs @@ -0,0 +1,24 @@ +// check-pass +#![deny(improper_ctypes)] + +use std::marker::PhantomData; + +trait Foo { + type Assoc; +} + +impl Foo for () { + type Assoc = PhantomData<()>; +} + +#[repr(transparent)] +struct Wow where T: Foo> { + x: ::Assoc, + v: u32, +} + +extern "C" { + fn test(v: Wow<()>); +} + +fn main() {}