From 05d6cdb52c267e044a226f89322d1f127ec9a61a Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sat, 24 Jan 2026 13:37:53 +0100 Subject: [PATCH 1/4] ImproperCTypes: move ADT logic into separate visit_ methods Another interal change that shouldn't impact rustc users. The goal (of this commit and a few afterwards) is to break apart the gigantic visit_type function into more managable and easily-editable bits that focus on specific parts of FFI safety. For now, we break the code specific to enums one side, and structs/unions on the other, into separate visit_? methods --- .../rustc_lint/src/types/improper_ctypes.rs | 209 ++++++++++-------- 1 file changed, 115 insertions(+), 94 deletions(-) diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 965cd23c762ae..5be1ed1b000ee 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -190,6 +190,7 @@ fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { fn check_arg_for_power_alignment<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { let tcx = cx.tcx; assert!(tcx.sess.target.os == Os::Aix); + // Structs (under repr(C)) follow the power alignment rule if: // - the first field of the struct is a floating-point type that // is greater than 4-bytes, or @@ -377,15 +378,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } /// Checks if the given `VariantDef`'s field types are "ffi-safe". - fn check_variant_for_ffi( + fn visit_variant_fields( &mut self, state: VisitorState, ty: Ty<'tcx>, - def: ty::AdtDef<'tcx>, + def: AdtDef<'tcx>, variant: &ty::VariantDef, args: GenericArgsRef<'tcx>, ) -> FfiResult<'tcx> { use FfiResult::*; + let transparent_with_all_zst_fields = if def.repr().transparent() { if let Some(field) = super::transparent_newtype_field(self.cx.tcx, variant) { // Transparent newtypes have at most one non-ZST field which needs to be checked.. @@ -431,6 +433,115 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } + fn visit_struct_or_union( + &mut self, + state: VisitorState, + ty: Ty<'tcx>, + def: AdtDef<'tcx>, + args: GenericArgsRef<'tcx>, + ) -> FfiResult<'tcx> { + debug_assert!(matches!(def.adt_kind(), AdtKind::Struct | AdtKind::Union)); + use FfiResult::*; + + if !def.repr().c() && !def.repr().transparent() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + msg!("this struct has unspecified layout") + } else { + msg!("this union has unspecified layout") + }, + help: if def.is_struct() { + Some(msg!( + "consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct" + )) + } else { + // FIXME(ctypes): confirm that this makes sense for unions once #60405 / RFC2645 stabilises + Some(msg!( + "consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this union" + )) + }, + }; + } + + if def.non_enum_variant().field_list_has_applicable_non_exhaustive() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + msg!("this struct is non-exhaustive") + } else { + msg!("this union is non-exhaustive") + }, + help: None, + }; + } + + if def.non_enum_variant().fields.is_empty() { + return FfiUnsafe { + ty, + reason: if def.is_struct() { + msg!("this struct has no fields") + } else { + msg!("this union has no fields") + }, + help: if def.is_struct() { + Some(msg!("consider adding a member to this struct")) + } else { + Some(msg!("consider adding a member to this union")) + }, + }; + } + self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args) + } + + fn visit_enum( + &mut self, + state: VisitorState, + ty: Ty<'tcx>, + def: AdtDef<'tcx>, + args: GenericArgsRef<'tcx>, + ) -> FfiResult<'tcx> { + debug_assert!(matches!(def.adt_kind(), AdtKind::Enum)); + use FfiResult::*; + + if def.variants().is_empty() { + // Empty enums are okay... although sort of useless. + return FfiSafe; + } + // Check for a repr() attribute to specify the size of the discriminant. + if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() { + // Special-case types like `Option` and `Result` + if let Some(ty) = repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { + return self.visit_type(state, ty); + } + + return FfiUnsafe { + ty, + reason: msg!("enum has no representation hint"), + help: Some(msg!( + "consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum" + )), + }; + } + + let non_exhaustive = def.variant_list_has_applicable_non_exhaustive(); + // Check the contained variants. + let ret = def.variants().iter().try_for_each(|variant| { + check_non_exhaustive_variant(non_exhaustive, variant) + .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; + + match self.visit_variant_fields(state, ty, def, variant, args) { + FfiSafe => ControlFlow::Continue(()), + r => ControlFlow::Break(r), + } + }); + if let ControlFlow::Break(result) = ret { + return result; + } + + FfiSafe + } + /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). fn visit_type(&mut self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> { @@ -483,99 +594,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { )), }; } - - if !def.repr().c() && !def.repr().transparent() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - msg!("this struct has unspecified layout") - } else { - msg!("this union has unspecified layout") - }, - help: if def.is_struct() { - Some(msg!( - "consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct" - )) - } else { - Some(msg!( - "consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this union" - )) - }, - }; - } - - if def.non_enum_variant().field_list_has_applicable_non_exhaustive() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - msg!("this struct is non-exhaustive") - } else { - msg!("this union is non-exhaustive") - }, - help: None, - }; - } - - if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe { - ty, - reason: if def.is_struct() { - msg!("this struct has no fields") - } else { - msg!("this union has no fields") - }, - help: if def.is_struct() { - Some(msg!("consider adding a member to this struct")) - } else { - Some(msg!("consider adding a member to this union")) - }, - }; - } - - self.check_variant_for_ffi(state, ty, def, def.non_enum_variant(), args) - } - AdtKind::Enum => { - if def.variants().is_empty() { - // Empty enums are okay... although sort of useless. - return FfiSafe; - } - // Check for a repr() attribute to specify the size of the - // discriminant. - if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() - { - // Special-case types like `Option` and `Result` - if let Some(ty) = - repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) - { - return self.visit_type(state, ty); - } - - return FfiUnsafe { - ty, - reason: msg!("enum has no representation hint"), - help: Some(msg!( - "consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum" - )), - }; - } - - let non_exhaustive = def.variant_list_has_applicable_non_exhaustive(); - // Check the contained variants. - let ret = def.variants().iter().try_for_each(|variant| { - check_non_exhaustive_variant(non_exhaustive, variant) - .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; - - match self.check_variant_for_ffi(state, ty, def, variant, args) { - FfiSafe => ControlFlow::Continue(()), - r => ControlFlow::Break(r), - } - }); - if let ControlFlow::Break(result) = ret { - return result; - } - - FfiSafe + self.visit_struct_or_union(state, ty, def, args) } + AdtKind::Enum => self.visit_enum(state, ty, def, args), } } From d75c93dad24c17ce384e55d7824c9f4d0b2f158a Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sat, 24 Jan 2026 13:37:53 +0100 Subject: [PATCH 2/4] ImproperCTypes: reorder simple type checks Another interal change that shouldn't impact rustc users. the list of simpler type-based decisions made by `visit_type` are reordered and are given better documentation. --- .../rustc_lint/src/types/improper_ctypes.rs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 5be1ed1b000ee..54dd54451d847 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -600,19 +600,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } } - ty::Char => FfiUnsafe { + // Pattern types are just extra invariants on the type that you need to uphold, + // but only the base type is relevant for being representable in FFI. + // (note: this lint was written when pattern types could only be integers constrained to ranges) + ty::Pat(pat_ty, _) => self.visit_type(state, pat_ty), + + // types which likely have a stable representation, if the target architecture defines those + // note: before rust 1.77, 128-bit ints were not FFI-safe on x86_64 + ty::Int(..) | ty::Uint(..) | ty::Float(..) => FfiResult::FfiSafe, + + ty::Bool => FfiResult::FfiSafe, + + ty::Char => FfiResult::FfiUnsafe { ty, reason: msg!("the `char` type has no C equivalent"), help: Some(msg!("consider using `u32` or `libc::wchar_t` instead")), }, - // It's just extra invariants on the type that you need to uphold, - // but only the base type is relevant for being representable in FFI. - ty::Pat(base, ..) => self.visit_type(state, base), - - // Primitive types with a stable representation. - ty::Bool | ty::Int(..) | ty::Uint(..) | ty::Float(..) | ty::Never => FfiSafe, - ty::Slice(_) => FfiUnsafe { ty, reason: msg!("slices have no C equivalent"), @@ -687,6 +691,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Foreign(..) => FfiSafe, + ty::Never => 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::Alias(ty::AliasTy { kind: ty::Opaque { .. }, .. }) => { From 6dfa52ca93fb132e039f242801d9f3cab8c01863 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sat, 24 Jan 2026 13:37:53 +0100 Subject: [PATCH 3/4] ImproperCTypes: regroup Box/Ptr/Ref logic Another interal change that shouldn't impact rustc users. This time, regroup into `visit_indirections` the code dealing with the FFI safety of Boxes, Refs and RawPtrs. --- .../rustc_lint/src/types/improper_ctypes.rs | 125 +++++++++++++----- 1 file changed, 95 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 54dd54451d847..123b62cfdfe94 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -272,6 +272,17 @@ enum FfiResult<'tcx> { /// in the `FfiResult` is final. type PartialFfiResult<'tcx> = Option>; +/// What type indirection points to a given type. +#[derive(Clone, Copy)] +enum IndirectionKind { + /// Box (valid non-null pointer, owns pointee). + Box, + /// Ref (valid non-null pointer, borrows pointee). + Ref, + /// Raw pointer (not necessarily non-null or valid. no info on ownership). + RawPtr, +} + bitflags! { #[derive(Clone, Copy, Debug, PartialEq, Eq)] struct VisitorState: u8 { @@ -377,6 +388,78 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { Self { cx, base_ty, base_fn_mode, cache: FxHashSet::default() } } + /// Checks if the given indirection (box,ref,pointer) is "ffi-safe". + fn visit_indirection( + &mut self, + state: VisitorState, + ty: Ty<'tcx>, + inner_ty: Ty<'tcx>, + indirection_kind: IndirectionKind, + ) -> FfiResult<'tcx> { + use FfiResult::*; + let tcx = self.cx.tcx; + + match indirection_kind { + IndirectionKind::Box => { + // FIXME(ctypes): this logic is broken, but it still fits the current tests: + // - for some reason `Box<_>`es in `extern "ABI" {}` blocks + // (including within FnPtr:s) + // are not treated as pointers but as FFI-unsafe structs + // - otherwise, treat the box itself correctly, and follow pointee safety logic + // as described in the other `indirection_type` match branch. + if state.is_in_defined_function() + || (state.is_in_fnptr() && matches!(self.base_fn_mode, CItemKind::Definition)) + { + if inner_ty.is_sized(tcx, self.cx.typing_env()) { + return FfiSafe; + } else { + return FfiUnsafe { + ty, + reason: msg!("box cannot be represented as a single pointer"), + help: None, + }; + } + } else { + // (mid-retcon-commit-chain comment:) + // this is the original fallback behavior, which is wrong + if let ty::Adt(def, args) = ty.kind() { + self.visit_struct_or_union(state, ty, *def, args) + } else if cfg!(debug_assertions) { + bug!("ImproperCTypes: this retcon commit was badly written") + } else { + FfiSafe + } + } + } + IndirectionKind::Ref | IndirectionKind::RawPtr => { + // Weird behaviour for pointee safety. the big question here is + // "if you have a FFI-unsafe pointee behind a FFI-safe pointer type, is it ok?" + // The answer until now is: + // "It's OK for rust-defined functions and callbacks, we'll assume those are + // meant to be opaque types on the other side of the FFI boundary". + // + // Reasoning: + // For extern function declarations, the actual definition of the function is + // written somewhere else, meaning the declaration is free to express this + // opaqueness with an extern type (opaque caller-side) or a std::ffi::c_void + // (opaque callee-side). For extern function definitions, however, in the case + // where the type is opaque caller-side, it is not opaque callee-side, + // and having the full type information is necessary to compile the function. + // + // It might be better to rething this, or even ignore pointee safety for a first + // batch of behaviour changes. See the discussion that ends with + // https://github.com/rust-lang/rust/pull/134697#issuecomment-2692610258 + if (state.is_in_defined_function() || state.is_in_fnptr()) + && inner_ty.is_sized(self.cx.tcx, self.cx.typing_env()) + { + FfiSafe + } else { + self.visit_type(state, inner_ty) + } + } + } + } + /// Checks if the given `VariantDef`'s field types are "ffi-safe". fn visit_variant_fields( &mut self, @@ -477,7 +560,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } if def.non_enum_variant().fields.is_empty() { - return FfiUnsafe { + FfiUnsafe { ty, reason: if def.is_struct() { msg!("this struct has no fields") @@ -489,9 +572,10 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } else { Some(msg!("consider adding a member to this union")) }, - }; + } + } else { + self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args) } - self.visit_variant_fields(state, ty, def, def.non_enum_variant(), args) } fn visit_enum( @@ -559,23 +643,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match *ty.kind() { ty::Adt(def, args) => { - if let Some(boxed) = ty.boxed_ty() - && ( - // FIXME(ctypes): this logic is broken, but it still fits the current tests - state.is_in_defined_function() - || (state.is_in_fnptr() - && matches!(self.base_fn_mode, CItemKind::Definition)) - ) - { - if boxed.is_sized(tcx, self.cx.typing_env()) { - return FfiSafe; - } else { - return FfiUnsafe { - ty, - reason: msg!("box cannot be represented as a single pointer"), - help: None, - }; - } + if let Some(inner_ty) = ty.boxed_ty() { + return self.visit_indirection(state, ty, inner_ty, IndirectionKind::Box); } if def.is_phantom_data() { return FfiPhantom(ty); @@ -639,15 +708,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { help: Some(msg!("consider using a struct instead")), }, - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) - if { - (state.is_in_defined_function() || state.is_in_fnptr()) - && ty.is_sized(self.cx.tcx, self.cx.typing_env()) - } => - { - FfiSafe - } - ty::RawPtr(ty, _) if match ty.kind() { ty::Tuple(tuple) => tuple.is_empty(), @@ -657,7 +717,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe } - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.visit_type(state, ty), + ty::RawPtr(inner_ty, _) => { + return self.visit_indirection(state, ty, inner_ty, IndirectionKind::RawPtr); + } + ty::Ref(_, inner_ty, _) => { + return self.visit_indirection(state, ty, inner_ty, IndirectionKind::Ref); + } ty::Array(inner_ty, _) => self.visit_type(state, inner_ty), From 7bb9851ae8f0825109f0529330334f4df9c029f3 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sat, 24 Jan 2026 13:37:53 +0100 Subject: [PATCH 4/4] ImproperCTypes: remove special cases through better state tracking Another interal change that shouldn't impact rustc users. Code called outside of `visit_type` (and callees) is moved inside, by adding new types to properly track the state of a type visitation. - OuterTyKind tracks the knowledge of the type "directly outside of" the one being visited (if we are visiting a struct's field, an array's element, etc) - RootUseFlags tracks the knowledge of how the "original type being visited" is used: static variable, function argument/return, etc. --- .../rustc_lint/src/types/improper_ctypes.rs | 135 +++++++++++------- 1 file changed, 85 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs index 123b62cfdfe94..cfcd199ed520d 100644 --- a/compiler/rustc_lint/src/types/improper_ctypes.rs +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -369,6 +369,35 @@ impl VisitorState { } } +bitflags! { + /// Data that summarises how an "outer type" surrounds its inner type(s) + #[derive(Clone, Copy, Debug, PartialEq, Eq)] + struct OuterTyData: u8 { + /// To show that there is no outer type, the current type is directly used by a `static` + /// variable or a function/FnPtr + const NO_OUTER_TY = 0b01; + /// For NO_OUTER_TY cases, show that we are being directly used by a FnPtr specifically + /// FIXME(ctypes): this is only used for "bad behaviour" reproduced for compatibility's sake + const NO_OUTER_TY_FNPTR = 0b10; + } +} + +impl OuterTyData { + /// Get the proper data for a given outer type. + fn from_ty<'tcx>(ty: Ty<'tcx>) -> Self { + match ty.kind() { + ty::FnPtr(..) => Self::NO_OUTER_TY | Self::NO_OUTER_TY_FNPTR, + ty::RawPtr(..) + | ty::Ref(..) + | ty::Adt(..) + | ty::Tuple(..) + | ty::Array(..) + | ty::Slice(_) => Self::empty(), + k @ _ => bug!("unexpected outer type {:?} of kind {:?}", ty, k), + } + } +} + /// Visitor used to recursively traverse MIR types and evaluate FFI-safety. /// It uses ``check_*`` methods as entrypoints to be called elsewhere, /// and ``visit_*`` methods to recurse. @@ -454,7 +483,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { { FfiSafe } else { - self.visit_type(state, inner_ty) + self.visit_type(state, OuterTyData::from_ty(ty), inner_ty) } } } @@ -475,7 +504,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if let Some(field) = super::transparent_newtype_field(self.cx.tcx, variant) { // Transparent newtypes have at most one non-ZST field which needs to be checked.. let field_ty = get_type_from_field(self.cx, field, args); - match self.visit_type(state, field_ty) { + match self.visit_type(state, OuterTyData::from_ty(ty), field_ty) { FfiUnsafe { ty, .. } if ty.is_unit() => (), r => return r, } @@ -494,7 +523,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let mut all_phantom = !variant.fields.is_empty(); for field in &variant.fields { let field_ty = get_type_from_field(self.cx, field, args); - all_phantom &= match self.visit_type(state, field_ty) { + all_phantom &= match self.visit_type(state, OuterTyData::from_ty(ty), field_ty) { FfiSafe => false, // `()` fields are FFI-safe! FfiUnsafe { ty, .. } if ty.is_unit() => false, @@ -592,11 +621,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // Empty enums are okay... although sort of useless. return FfiSafe; } - // Check for a repr() attribute to specify the size of the discriminant. + // Check for a repr() attribute to specify the size of the + // discriminant. if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() { // Special-case types like `Option` and `Result` - if let Some(ty) = repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { - return self.visit_type(state, ty); + if let Some(inner_ty) = repr_nullable_ptr(self.cx.tcx, self.cx.typing_env(), ty) { + return self.visit_type(state, OuterTyData::from_ty(ty), inner_ty); } return FfiUnsafe { @@ -628,7 +658,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Checks if the given type is "ffi-safe" (has a stable, well-defined /// representation which can be exported to C code). - fn visit_type(&mut self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> { + fn visit_type( + &mut self, + state: VisitorState, + outer_ty: OuterTyData, + ty: Ty<'tcx>, + ) -> FfiResult<'tcx> { use FfiResult::*; let tcx = self.cx.tcx; @@ -672,7 +707,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // Pattern types are just extra invariants on the type that you need to uphold, // but only the base type is relevant for being representable in FFI. // (note: this lint was written when pattern types could only be integers constrained to ranges) - ty::Pat(pat_ty, _) => self.visit_type(state, pat_ty), + ty::Pat(pat_ty, _) => self.visit_type(state, outer_ty, pat_ty), // types which likely have a stable representation, if the target architecture defines those // note: before rust 1.77, 128-bit ints were not FFI-safe on x86_64 @@ -702,11 +737,22 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { help: Some(msg!("consider using `*const u8` and a length instead")), }, - ty::Tuple(..) => FfiUnsafe { - ty, - reason: msg!("tuples have unspecified layout"), - help: Some(msg!("consider using a struct instead")), - }, + ty::Tuple(tuple) => { + // C functions can return void + let empty_and_safe = tuple.is_empty() + && outer_ty.contains(OuterTyData::NO_OUTER_TY) + && state.is_in_function_return(); + + if empty_and_safe { + FfiSafe + } else { + FfiUnsafe { + ty, + reason: msg!("tuples have unspecified layout"), + help: Some(msg!("consider using a struct instead")), + } + } + } ty::RawPtr(ty, _) if match ty.kind() { @@ -724,7 +770,25 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return self.visit_indirection(state, ty, inner_ty, IndirectionKind::Ref); } - ty::Array(inner_ty, _) => self.visit_type(state, inner_ty), + ty::Array(inner_ty, _) => { + if state.is_in_function() + && outer_ty.contains(OuterTyData::NO_OUTER_TY) + // FIXME(ctypes): VVV-this-VVV shouldn't be the case + && !outer_ty.contains(OuterTyData::NO_OUTER_TY_FNPTR) + { + // C doesn't really support passing arrays by value - the only way to pass an array by value + // is through a struct. + FfiResult::FfiUnsafe { + ty, + reason: msg!("passing raw arrays by value is not FFI-safe"), + help: Some(msg!("consider passing a pointer to the array")), + } + } else { + // let's allow phantoms to go through, + // since an array of 1-ZSTs is also a 1-ZST + self.visit_type(state, OuterTyData::from_ty(ty), inner_ty) + } + } ty::FnPtr(sig_tys, hdr) => { let sig = sig_tys.with(hdr); @@ -740,18 +804,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { let sig = tcx.instantiate_bound_regions_with_erased(sig); for arg in sig.inputs() { - match self.visit_type(VisitorState::ARGUMENT_TY_IN_FNPTR, *arg) { + match self.visit_type( + VisitorState::ARGUMENT_TY_IN_FNPTR, + OuterTyData::from_ty(ty), + *arg, + ) { FfiSafe => {} r => return r, } } let ret_ty = sig.output(); - if ret_ty.is_unit() { - return FfiSafe; - } - self.visit_type(VisitorState::RETURN_TY_IN_FNPTR, ret_ty) + self.visit_type(VisitorState::RETURN_TY_IN_FNPTR, OuterTyData::from_ty(ty), ret_ty) } ty::Foreign(..) => FfiSafe, @@ -819,43 +884,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }) } - /// Check if the type is array and emit an unsafe type lint. - fn check_for_array_ty(&mut self, ty: Ty<'tcx>) -> PartialFfiResult<'tcx> { - if let ty::Array(..) = ty.kind() { - Some(FfiResult::FfiUnsafe { - ty, - reason: msg!("passing raw arrays by value is not FFI-safe"), - help: Some(msg!("consider passing a pointer to the array")), - }) - } else { - None - } - } - - /// Determine the FFI-safety of a single (MIR) type, given the context of how it is used. fn check_type(&mut self, state: VisitorState, ty: Ty<'tcx>) -> FfiResult<'tcx> { let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.typing_env(), ty).unwrap_or(ty); if let Some(res) = self.visit_for_opaque_ty(ty) { return res; } - // C doesn't really support passing arrays by value - the only way to pass an array by value - // is through a struct. So, first test that the top level isn't an array, and then - // recursively check the types inside. - if state.is_in_function() { - if let Some(res) = self.check_for_array_ty(ty) { - return res; - } - } - - // Don't report FFI errors for unit return types. This check exists here, and not in - // the caller (where it would make more sense) so that normalization has definitely - // happened. - if state.is_in_function_return() && ty.is_unit() { - return FfiResult::FfiSafe; - } - - self.visit_type(state, ty) + self.visit_type(state, OuterTyData::NO_OUTER_TY, ty) } }