diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index a45817beea1..06e7c2b6f36 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -20,7 +20,9 @@ //! If you define a new `LateLintPass`, you will also need to add it to the //! `late_lint_methods!` invocation in `lib.rs`. -use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; +use crate::{ + types::CItemKind, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext, +}; use rustc_ast::ast::{self, Expr}; use rustc_ast::attr::{self, HasAttrs}; use rustc_ast::tokenstream::{TokenStream, TokenTree}; @@ -36,14 +38,14 @@ use rustc_hir::def_id::DefId; use rustc_hir::{ForeignItemKind, GenericParamKind, PatKind}; use rustc_hir::{HirId, HirIdSet, Node}; use rustc_middle::lint::LintDiagnosticBuilder; -use rustc_middle::ty::subst::GenericArgKind; +use rustc_middle::ty::subst::{GenericArgKind, Subst}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::lint::FutureIncompatibleInfo; use rustc_span::edition::Edition; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, Span}; -use rustc_target::abi::VariantIdx; +use rustc_target::abi::{LayoutOf, VariantIdx}; use rustc_trait_selection::traits::misc::can_type_implement_copy; use crate::nonstandard_style::{method_context, MethodLateContext}; @@ -2144,7 +2146,13 @@ impl ClashingExternDeclarations { /// Checks whether two types are structurally the same enough that the declarations shouldn't /// clash. We need this so we don't emit a lint when two modules both declare an extern struct, /// with the same members (as the declarations shouldn't clash). - fn structurally_same_type<'tcx>(cx: &LateContext<'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> bool { + fn structurally_same_type<'tcx>( + cx: &LateContext<'tcx>, + a: Ty<'tcx>, + b: Ty<'tcx>, + ckind: CItemKind, + ) -> bool { + debug!("structurally_same_type(cx, a = {:?}, b = {:?})", a, b); let tcx = cx.tcx; if a == b || rustc_middle::ty::TyS::same_type(a, b) { // All nominally-same types are structurally same, too. @@ -2155,47 +2163,77 @@ impl ClashingExternDeclarations { let a_kind = &a.kind; let b_kind = &b.kind; + let compare_layouts = |a, b| -> bool { + let a_layout = &cx.layout_of(a).unwrap().layout.abi; + let b_layout = &cx.layout_of(b).unwrap().layout.abi; + debug!("{:?} == {:?} = {}", a_layout, b_layout, a_layout == b_layout); + a_layout == b_layout + }; + + #[allow(rustc::usage_of_ty_tykind)] + let is_primitive_or_pointer = + |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..)); + match (a_kind, b_kind) { - (Adt(..), Adt(..)) => { - // Adts are pretty straightforward: just compare the layouts. - use rustc_target::abi::LayoutOf; - let a_layout = cx.layout_of(a).unwrap().layout; - let b_layout = cx.layout_of(b).unwrap().layout; - a_layout == b_layout + (Adt(_, a_substs), Adt(_, b_substs)) => { + let a = a.subst(cx.tcx, a_substs); + let b = b.subst(cx.tcx, b_substs); + debug!("Comparing {:?} and {:?}", a, b); + + if let (Adt(a_def, ..), Adt(b_def, ..)) = (&a.kind, &b.kind) { + // Grab a flattened representation of all fields. + let a_fields = a_def.variants.iter().flat_map(|v| v.fields.iter()); + let b_fields = b_def.variants.iter().flat_map(|v| v.fields.iter()); + compare_layouts(a, b) + && a_fields.eq_by( + b_fields, + |&ty::FieldDef { did: a_did, .. }, + &ty::FieldDef { did: b_did, .. }| { + Self::structurally_same_type( + cx, + tcx.type_of(a_did), + tcx.type_of(b_did), + ckind, + ) + }, + ) + } else { + unreachable!() + } } (Array(a_ty, a_const), Array(b_ty, b_const)) => { // For arrays, we also check the constness of the type. a_const.val == b_const.val - && Self::structurally_same_type(cx, a_const.ty, b_const.ty) - && Self::structurally_same_type(cx, a_ty, b_ty) + && Self::structurally_same_type(cx, a_const.ty, b_const.ty, ckind) + && Self::structurally_same_type(cx, a_ty, b_ty, ckind) } - (Slice(a_ty), Slice(b_ty)) => Self::structurally_same_type(cx, a_ty, b_ty), + (Slice(a_ty), Slice(b_ty)) => Self::structurally_same_type(cx, a_ty, b_ty, ckind), (RawPtr(a_tymut), RawPtr(b_tymut)) => { a_tymut.mutbl == a_tymut.mutbl - && Self::structurally_same_type(cx, &a_tymut.ty, &b_tymut.ty) + && Self::structurally_same_type(cx, &a_tymut.ty, &b_tymut.ty, ckind) } (Ref(_a_region, a_ty, a_mut), Ref(_b_region, b_ty, b_mut)) => { // For structural sameness, we don't need the region to be same. - a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty) + a_mut == b_mut && Self::structurally_same_type(cx, a_ty, b_ty, ckind) } (FnDef(..), FnDef(..)) => { - // As we don't compare regions, skip_binder is fine. let a_poly_sig = a.fn_sig(tcx); let b_poly_sig = b.fn_sig(tcx); + // As we don't compare regions, skip_binder is fine. let a_sig = a_poly_sig.skip_binder(); let b_sig = b_poly_sig.skip_binder(); (a_sig.abi, a_sig.unsafety, a_sig.c_variadic) == (b_sig.abi, b_sig.unsafety, b_sig.c_variadic) && a_sig.inputs().iter().eq_by(b_sig.inputs().iter(), |a, b| { - Self::structurally_same_type(cx, a, b) + Self::structurally_same_type(cx, a, b, ckind) }) - && Self::structurally_same_type(cx, a_sig.output(), b_sig.output()) + && Self::structurally_same_type(cx, a_sig.output(), b_sig.output(), ckind) } (Tuple(a_substs), Tuple(b_substs)) => { a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| { - Self::structurally_same_type(cx, a_ty, b_ty) + Self::structurally_same_type(cx, a_ty, b_ty, ckind) }) } // For these, it's not quite as easy to define structural-sameness quite so easily. @@ -2208,9 +2246,27 @@ impl ClashingExternDeclarations { | (GeneratorWitness(..), GeneratorWitness(..)) | (Projection(..), Projection(..)) | (Opaque(..), Opaque(..)) => false, + // These definitely should have been caught above. (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - _ => false, + + // An Adt and a primitive type. This can be FFI-safe is the ADT is an enum with a + // non-null field. + (Adt(..), other_kind) | (other_kind, Adt(..)) + if is_primitive_or_pointer(other_kind) => + { + let (primitive, adt) = + if is_primitive_or_pointer(&a.kind) { (a, b) } else { (b, a) }; + if let Some(ty) = crate::types::repr_nullable_ptr(cx, adt, ckind) { + ty == primitive + } else { + compare_layouts(a, b) + } + } + // Otherwise, just compare the layouts. This may fail to lint for some + // incompatible types, but at the very least, will stop reads into + // uninitialised memory. + _ => compare_layouts(a, b), } } } @@ -2231,7 +2287,12 @@ impl<'tcx> LateLintPass<'tcx> for ClashingExternDeclarations { existing_hid, existing_decl_ty, this_fi.hir_id, this_decl_ty ); // Check that the declarations match. - if !Self::structurally_same_type(cx, existing_decl_ty, this_decl_ty) { + if !Self::structurally_same_type( + cx, + existing_decl_ty, + this_decl_ty, + CItemKind::Declaration, + ) { let orig_fi = tcx.hir().expect_foreign_item(existing_hid); let orig = Self::name_of_extern_decl(tcx, orig_fi); diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 1affc9457b8..de750010ed1 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -15,8 +15,9 @@ use rustc_middle::ty::{self, AdtKind, Ty, TypeFoldable}; use rustc_span::source_map; use rustc_span::symbol::sym; use rustc_span::{Span, DUMMY_SP}; +use rustc_target::abi::Abi; use rustc_target::abi::{Integer, LayoutOf, TagEncoding, VariantIdx, Variants}; -use rustc_target::spec::abi::Abi; +use rustc_target::spec::abi::Abi as SpecAbi; use log::debug; use std::cmp; @@ -509,14 +510,15 @@ declare_lint! { declare_lint_pass!(ImproperCTypesDefinitions => [IMPROPER_CTYPES_DEFINITIONS]); -enum ImproperCTypesMode { - Declarations, - Definitions, +#[derive(Clone, Copy)] +crate enum CItemKind { + Declaration, + Definition, } struct ImproperCTypesVisitor<'a, 'tcx> { cx: &'a LateContext<'tcx>, - mode: ImproperCTypesMode, + mode: CItemKind, } enum FfiResult<'tcx> { @@ -525,54 +527,94 @@ enum FfiResult<'tcx> { FfiUnsafe { ty: Ty<'tcx>, reason: String, help: Option }, } -impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { - /// Is type known to be non-null? - fn ty_is_known_nonnull(&self, ty: Ty<'tcx>) -> bool { - match ty.kind { - ty::FnPtr(_) => true, - ty::Ref(..) => true, - ty::Adt(def, _) - if def.is_box() && matches!(self.mode, ImproperCTypesMode::Definitions) => - { - true +/// Is type known to be non-null? +fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool { + let tcx = cx.tcx; + match ty.kind { + ty::FnPtr(_) => true, + ty::Ref(..) => true, + ty::Adt(def, _) if def.is_box() && matches!(mode, CItemKind::Definition) => true, + ty::Adt(def, substs) if def.repr.transparent() && !def.is_union() => { + let guaranteed_nonnull_optimization = tcx + .get_attrs(def.did) + .iter() + .any(|a| a.check_name(sym::rustc_nonnull_optimization_guaranteed)); + + if guaranteed_nonnull_optimization { + return true; } - ty::Adt(def, substs) if def.repr.transparent() && !def.is_union() => { - let guaranteed_nonnull_optimization = self - .cx - .tcx - .get_attrs(def.did) - .iter() - .any(|a| a.check_name(sym::rustc_nonnull_optimization_guaranteed)); - - if guaranteed_nonnull_optimization { - return true; - } - - for variant in &def.variants { - if let Some(field) = variant.transparent_newtype_field(self.cx.tcx) { - if self.ty_is_known_nonnull(field.ty(self.cx.tcx, substs)) { - return true; - } + for variant in &def.variants { + if let Some(field) = variant.transparent_newtype_field(tcx) { + if ty_is_known_nonnull(cx, field.ty(tcx, substs), mode) { + return true; } } - - false } - _ => false, - } - } - /// Check if this enum can be safely exported based on the "nullable pointer optimization". - /// Currently restricted to function pointers, boxes, references, `core::num::NonZero*`, - /// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes. - fn is_repr_nullable_ptr( - &self, - ty: Ty<'tcx>, - ty_def: &'tcx ty::AdtDef, - substs: SubstsRef<'tcx>, - ) -> bool { + false + } + _ => false, + } +} +/// Given a non-null scalar (or transparent) type `ty`, return the nullable version of that type. +/// If the type passed in was not scalar, returns None. +fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option> { + let tcx = cx.tcx; + Some(match ty.kind { + ty::Adt(field_def, field_substs) => { + let inner_field_ty = { + let first_non_zst_ty = + field_def.variants.iter().filter_map(|v| v.transparent_newtype_field(tcx)); + debug_assert_eq!( + first_non_zst_ty.clone().count(), + 1, + "Wrong number of fields for transparent type" + ); + first_non_zst_ty + .last() + .expect("No non-zst fields in transparent type.") + .ty(tcx, field_substs) + }; + return get_nullable_type(cx, inner_field_ty); + } + ty::Int(ty) => tcx.mk_mach_int(ty), + ty::Uint(ty) => tcx.mk_mach_uint(ty), + ty::RawPtr(ty_mut) => tcx.mk_ptr(ty_mut), + // As these types are always non-null, the nullable equivalent of + // Option of these types are their raw pointer counterparts. + ty::Ref(_region, ty, mutbl) => tcx.mk_ptr(ty::TypeAndMut { ty, mutbl }), + ty::FnPtr(..) => { + // There is no nullable equivalent for Rust's function pointers -- you + // must use an Option _> to represent it. + ty + } + + // We should only ever reach this case if ty_is_known_nonnull is extended + // to other types. + ref unhandled => { + debug!( + "get_nullable_type: Unhandled scalar kind: {:?} while checking {:?}", + unhandled, ty + ); + return None; + } + }) +} + +/// Check if this enum can be safely exported based on the "nullable pointer optimization". If it +/// can, return the the type that `ty` can be safely converted to, otherwise return `None`. +/// Currently restricted to function pointers, boxes, references, `core::num::NonZero*`, +/// `core::ptr::NonNull`, and `#[repr(transparent)]` newtypes. +/// FIXME: This duplicates code in codegen. +crate fn repr_nullable_ptr<'tcx>( + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + ckind: CItemKind, +) -> Option> { + debug!("is_repr_nullable_ptr(cx, ty = {:?})", ty); + if let ty::Adt(ty_def, substs) = ty.kind { if ty_def.variants.len() != 2 { - return false; + return None; } let get_variant_fields = |index| &ty_def.variants[VariantIdx::new(index)].fields; @@ -582,30 +624,42 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } else if variant_fields[1].is_empty() { &variant_fields[0] } else { - return false; + return None; }; if fields.len() != 1 { - return false; + return None; } - let field_ty = fields[0].ty(self.cx.tcx, substs); - if !self.ty_is_known_nonnull(field_ty) { - return false; + let field_ty = fields[0].ty(cx.tcx, substs); + if !ty_is_known_nonnull(cx, field_ty, ckind) { + return None; } - // At this point, the field's type is known to be nonnull and the parent enum is - // Option-like. If the computed size for the field and the enum are different, the non-null - // optimization isn't being applied (and we've got a problem somewhere). - let compute_size_skeleton = - |t| SizeSkeleton::compute(t, self.cx.tcx, self.cx.param_env).unwrap(); + // At this point, the field's type is known to be nonnull and the parent enum is Option-like. + // If the computed size for the field and the enum are different, the nonnull optimization isn't + // being applied (and we've got a problem somewhere). + let compute_size_skeleton = |t| SizeSkeleton::compute(t, cx.tcx, cx.param_env).unwrap(); if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) { bug!("improper_ctypes: Option nonnull optimization not applied?"); } - true + // Return the nullable type this Option-like enum can be safely represented with. + let field_ty_abi = &cx.layout_of(field_ty).unwrap().abi; + if let Abi::Scalar(field_ty_scalar) = field_ty_abi { + match (field_ty_scalar.valid_range.start(), field_ty_scalar.valid_range.end()) { + (0, _) => unreachable!("Non-null optimisation extended to a non-zero value."), + (1, _) => { + return Some(get_nullable_type(cx, field_ty).unwrap()); + } + (start, end) => unreachable!("Unhandled start and end range: ({}, {})", start, end), + }; + } } + None +} +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, sp: Span, ty: Ty<'tcx>) -> bool { if let ty::Array(..) = ty.kind { @@ -686,7 +740,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { fn check_type_for_ffi(&self, cache: &mut FxHashSet>, ty: Ty<'tcx>) -> FfiResult<'tcx> { use FfiResult::*; - let cx = self.cx.tcx; + let tcx = self.cx.tcx; // Protect against infinite recursion, for example // `struct S(*mut S);`. @@ -697,9 +751,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } match ty.kind { - ty::Adt(def, _) - if def.is_box() && matches!(self.mode, ImproperCTypesMode::Definitions) => - { + ty::Adt(def, _) if def.is_box() && matches!(self.mode, CItemKind::Definition) => { FfiSafe } @@ -753,7 +805,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // discriminant. if !def.repr.c() && !def.repr.transparent() && def.repr.int.is_none() { // Special-case types like `Option`. - if !self.is_repr_nullable_ptr(ty, def, substs) { + if repr_nullable_ptr(self.cx, ty, self.mode).is_none() { return FfiUnsafe { ty, reason: "enum has no representation hint".into(), @@ -836,7 +888,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::RawPtr(ty::TypeAndMut { ty, .. }) | ty::Ref(_, ty, _) if { - matches!(self.mode, ImproperCTypesMode::Definitions) + matches!(self.mode, CItemKind::Definition) && ty.is_sized(self.cx.tcx.at(DUMMY_SP), self.cx.param_env) } => { @@ -862,7 +914,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } - let sig = cx.erase_late_bound_regions(&sig); + let sig = tcx.erase_late_bound_regions(&sig); if !sig.output().is_unit() { let r = self.check_type_for_ffi(cache, sig.output()); match r { @@ -894,9 +946,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // `extern "C" fn` functions can have type parameters, which may or may not be FFI-safe, // so they are currently ignored for the purposes of this lint. - ty::Param(..) | ty::Projection(..) - if matches!(self.mode, ImproperCTypesMode::Definitions) => - { + ty::Param(..) | ty::Projection(..) if matches!(self.mode, CItemKind::Definition) => { FfiSafe } @@ -921,14 +971,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { help: Option<&str>, ) { let lint = match self.mode { - ImproperCTypesMode::Declarations => IMPROPER_CTYPES, - ImproperCTypesMode::Definitions => IMPROPER_CTYPES_DEFINITIONS, + CItemKind::Declaration => IMPROPER_CTYPES, + CItemKind::Definition => IMPROPER_CTYPES_DEFINITIONS, }; self.cx.struct_span_lint(lint, sp, |lint| { let item_description = match self.mode { - ImproperCTypesMode::Declarations => "block", - ImproperCTypesMode::Definitions => "fn", + CItemKind::Declaration => "block", + CItemKind::Definition => "fn", }; let mut diag = lint.build(&format!( "`extern` {} uses type `{}`, which is not FFI-safe", @@ -1052,8 +1102,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { self.check_type_for_ffi_and_report_errors(span, ty, true, false); } - fn is_internal_abi(&self, abi: Abi) -> bool { - if let Abi::Rust | Abi::RustCall | Abi::RustIntrinsic | Abi::PlatformIntrinsic = abi { + fn is_internal_abi(&self, abi: SpecAbi) -> bool { + if let SpecAbi::Rust + | SpecAbi::RustCall + | SpecAbi::RustIntrinsic + | SpecAbi::PlatformIntrinsic = abi + { true } else { false @@ -1063,7 +1117,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { fn check_foreign_item(&mut self, cx: &LateContext<'_>, it: &hir::ForeignItem<'_>) { - let mut vis = ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Declarations }; + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; let abi = cx.tcx.hir().get_foreign_abi(it.hir_id); if !vis.is_internal_abi(abi) { @@ -1098,7 +1152,7 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { _ => return, }; - let mut vis = ImproperCTypesVisitor { cx, mode: ImproperCTypesMode::Definitions }; + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; if !vis.is_internal_abi(abi) { vis.check_foreign_fn(hir_id, decl); } diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index df8fa4d73dd..310ab4f7235 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -202,6 +202,16 @@ pub enum TyKind<'tcx> { Error(DelaySpanBugEmitted), } +impl TyKind<'tcx> { + #[inline] + pub fn is_primitive(&self) -> bool { + match self { + Bool | Char | Int(_) | Uint(_) | Float(_) => true, + _ => false, + } + } +} + /// A type that is not publicly constructable. This prevents people from making `TyKind::Error` /// except through `tcx.err*()`. #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] @@ -1766,10 +1776,7 @@ impl<'tcx> TyS<'tcx> { #[inline] pub fn is_primitive(&self) -> bool { - match self.kind { - Bool | Char | Int(_) | Uint(_) | Float(_) => true, - _ => false, - } + self.kind.is_primitive() } #[inline] diff --git a/src/test/ui/lint/auxiliary/external_extern_fn.rs b/src/test/ui/lint/auxiliary/external_extern_fn.rs index b2caebc6fee..c2a8cadc6b5 100644 --- a/src/test/ui/lint/auxiliary/external_extern_fn.rs +++ b/src/test/ui/lint/auxiliary/external_extern_fn.rs @@ -1,3 +1,3 @@ -extern { +extern "C" { pub fn extern_fn(x: u8); } diff --git a/src/test/ui/lint/clashing-extern-fn.rs b/src/test/ui/lint/clashing-extern-fn.rs index 544614100ba..d6ac7ccccc7 100644 --- a/src/test/ui/lint/clashing-extern-fn.rs +++ b/src/test/ui/lint/clashing-extern-fn.rs @@ -3,52 +3,40 @@ #![crate_type = "lib"] #![warn(clashing_extern_declarations)] +mod redeclared_different_signature { + mod a { + extern "C" { + fn clash(x: u8); + } + } + mod b { + extern "C" { + fn clash(x: u64); //~ WARN `clash` redeclared with a different signature + } + } +} + +mod redeclared_same_signature { + mod a { + extern "C" { + fn no_clash(x: u8); + } + } + mod b { + extern "C" { + fn no_clash(x: u8); + } + } +} + extern crate external_extern_fn; - -extern "C" { - fn clash(x: u8); - fn no_clash(x: u8); -} - -fn redeclared_different_signature() { +mod extern_no_clash { + // Should not clash with external_extern_fn::extern_fn. extern "C" { - fn clash(x: u64); //~ WARN `clash` redeclared with a different signature - } - - unsafe { - clash(123); - no_clash(123); + fn extern_fn(x: u8); } } -fn redeclared_same_signature() { - extern "C" { - fn no_clash(x: u8); - } - unsafe { - no_clash(123); - } -} - -extern "C" { - fn extern_fn(x: u64); -} - -fn extern_clash() { - extern "C" { - fn extern_fn(x: u32); //~ WARN `extern_fn` redeclared with a different signature - } - unsafe { - extern_fn(123); - } -} - -fn extern_no_clash() { - unsafe { - external_extern_fn::extern_fn(123); - crate::extern_fn(123); - } -} extern "C" { fn some_other_new_name(x: i16); @@ -134,9 +122,9 @@ mod banana { weight: u32, length: u16, } // note: distinct type - extern "C" { // This should not trigger the lint because two::Banana is structurally equivalent to // one::Banana. + extern "C" { fn weigh_banana(count: *const Banana) -> u64; } } @@ -180,7 +168,120 @@ mod sameish_members { // always be the case, for every architecture and situation. This is also a really odd // thing to do anyway. extern "C" { - fn draw_point(p: Point); //~ WARN `draw_point` redeclared with a different + fn draw_point(p: Point); + //~^ WARN `draw_point` redeclared with a different signature + } + } +} + +mod same_sized_members_clash { + mod a { + #[repr(C)] + struct Point3 { + x: f32, + y: f32, + z: f32, + } + extern "C" { fn origin() -> Point3; } + } + mod b { + #[repr(C)] + struct Point3 { + x: i32, + y: i32, + z: i32, // NOTE: Incorrectly redeclared as i32 + } + extern "C" { fn origin() -> Point3; } + //~^ WARN `origin` redeclared with a different signature + } +} + +mod transparent { + #[repr(transparent)] + struct T(usize); + mod a { + use super::T; + extern "C" { + fn transparent() -> T; + fn transparent_incorrect() -> T; + } + } + + mod b { + extern "C" { + // Shouldn't warn here, because repr(transparent) guarantees that T's layout is the + // same as just the usize. + fn transparent() -> usize; + + // Should warn, because there's a signedness conversion here: + fn transparent_incorrect() -> isize; + //~^ WARN `transparent_incorrect` redeclared with a different signature + } + } +} + +mod missing_return_type { + mod a { + extern "C" { + fn missing_return_type() -> usize; + } + } + + mod b { + extern "C" { + // This should output a warning because we can't assume that the first declaration is + // the correct one -- if this one is the correct one, then calling the usize-returning + // version would allow reads into uninitialised memory. + fn missing_return_type(); + //~^ WARN `missing_return_type` redeclared with a different signature + } + } +} + +mod non_zero_and_non_null { + mod a { + extern "C" { + fn non_zero_usize() -> core::num::NonZeroUsize; + fn non_null_ptr() -> core::ptr::NonNull; + } + } + mod b { + extern "C" { + // If there's a clash in either of these cases you're either gaining an incorrect + // invariant that the value is non-zero, or you're missing out on that invariant. Both + // cases are warning for, from both a caller-convenience and optimisation perspective. + fn non_zero_usize() -> usize; + //~^ WARN `non_zero_usize` redeclared with a different signature + fn non_null_ptr() -> *const usize; + //~^ WARN `non_null_ptr` redeclared with a different signature + } + } +} + +mod null_optimised_enums { + mod a { + extern "C" { + fn option_non_zero_usize() -> usize; + fn option_non_zero_isize() -> isize; + fn option_non_null_ptr() -> *const usize; + + fn option_non_zero_usize_incorrect() -> usize; + fn option_non_null_ptr_incorrect() -> *const usize; + } + } + mod b { + extern "C" { + // This should be allowed, because these conversions are guaranteed to be FFI-safe (see + // #60300) + fn option_non_zero_usize() -> Option; + fn option_non_zero_isize() -> Option; + fn option_non_null_ptr() -> Option>; + + // However, these should be incorrect (note isize instead of usize) + fn option_non_zero_usize_incorrect() -> isize; + //~^ WARN `option_non_zero_usize_incorrect` redeclared with a different signature + fn option_non_null_ptr_incorrect() -> *const isize; + //~^ WARN `option_non_null_ptr_incorrect` redeclared with a different signature } } } diff --git a/src/test/ui/lint/clashing-extern-fn.stderr b/src/test/ui/lint/clashing-extern-fn.stderr index 96e51ab5a3e..cca0c4c59eb 100644 --- a/src/test/ui/lint/clashing-extern-fn.stderr +++ b/src/test/ui/lint/clashing-extern-fn.stderr @@ -1,11 +1,11 @@ warning: `clash` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:15:9 + --> $DIR/clashing-extern-fn.rs:14:13 | -LL | fn clash(x: u8); - | ---------------- `clash` previously declared here +LL | fn clash(x: u8); + | ---------------- `clash` previously declared here ... -LL | fn clash(x: u64); - | ^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration +LL | fn clash(x: u64); + | ^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration | note: the lint level is defined here --> $DIR/clashing-extern-fn.rs:4:9 @@ -15,20 +15,8 @@ LL | #![warn(clashing_extern_declarations)] = note: expected `unsafe extern "C" fn(u8)` found `unsafe extern "C" fn(u64)` -warning: `extern_fn` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:39:9 - | -LL | fn extern_fn(x: u64); - | --------------------- `extern_fn` previously declared here -... -LL | fn extern_fn(x: u32); - | ^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration - | - = note: expected `unsafe extern "C" fn(u64)` - found `unsafe extern "C" fn(u32)` - warning: `extern_link_name` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:64:9 + --> $DIR/clashing-extern-fn.rs:52:9 | LL | / #[link_name = "extern_link_name"] LL | | fn some_new_name(x: i16); @@ -41,7 +29,7 @@ LL | fn extern_link_name(x: u32); found `unsafe extern "C" fn(u32)` warning: `some_other_extern_link_name` redeclares `some_other_new_name` with a different signature - --> $DIR/clashing-extern-fn.rs:67:9 + --> $DIR/clashing-extern-fn.rs:55:9 | LL | fn some_other_new_name(x: i16); | ------------------------------- `some_other_new_name` previously declared here @@ -55,7 +43,7 @@ LL | | fn some_other_extern_link_name(x: u32); found `unsafe extern "C" fn(u32)` warning: `other_both_names_different` redeclares `link_name_same` with a different signature - --> $DIR/clashing-extern-fn.rs:71:9 + --> $DIR/clashing-extern-fn.rs:59:9 | LL | / #[link_name = "link_name_same"] LL | | fn both_names_different(x: i16); @@ -70,7 +58,7 @@ LL | | fn other_both_names_different(x: u32); found `unsafe extern "C" fn(u32)` warning: `different_mod` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:84:9 + --> $DIR/clashing-extern-fn.rs:72:9 | LL | fn different_mod(x: u8); | ------------------------ `different_mod` previously declared here @@ -82,7 +70,7 @@ LL | fn different_mod(x: u64); found `unsafe extern "C" fn(u64)` warning: `variadic_decl` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:94:9 + --> $DIR/clashing-extern-fn.rs:82:9 | LL | fn variadic_decl(x: u8, ...); | ----------------------------- `variadic_decl` previously declared here @@ -94,7 +82,7 @@ LL | fn variadic_decl(x: u8); found `unsafe extern "C" fn(u8)` warning: `weigh_banana` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:154:13 + --> $DIR/clashing-extern-fn.rs:142:13 | LL | fn weigh_banana(count: *const Banana) -> u64; | --------------------------------------------- `weigh_banana` previously declared here @@ -106,7 +94,7 @@ LL | fn weigh_banana(count: *const Banana) -> u64; found `unsafe extern "C" fn(*const banana::three::Banana) -> u64` warning: `draw_point` redeclared with a different signature - --> $DIR/clashing-extern-fn.rs:183:13 + --> $DIR/clashing-extern-fn.rs:171:13 | LL | fn draw_point(p: Point); | ------------------------ `draw_point` previously declared here @@ -117,5 +105,89 @@ LL | fn draw_point(p: Point); = note: expected `unsafe extern "C" fn(sameish_members::a::Point)` found `unsafe extern "C" fn(sameish_members::b::Point)` -warning: 9 warnings emitted +warning: `origin` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:194:22 + | +LL | extern "C" { fn origin() -> Point3; } + | ---------------------- `origin` previously declared here +... +LL | extern "C" { fn origin() -> Point3; } + | ^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> same_sized_members_clash::a::Point3` + found `unsafe extern "C" fn() -> same_sized_members_clash::b::Point3` + +warning: `transparent_incorrect` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:217:13 + | +LL | fn transparent_incorrect() -> T; + | -------------------------------- `transparent_incorrect` previously declared here +... +LL | fn transparent_incorrect() -> isize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> transparent::T` + found `unsafe extern "C" fn() -> isize` + +warning: `missing_return_type` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:235:13 + | +LL | fn missing_return_type() -> usize; + | ---------------------------------- `missing_return_type` previously declared here +... +LL | fn missing_return_type(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> usize` + found `unsafe extern "C" fn()` + +warning: `non_zero_usize` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:253:13 + | +LL | fn non_zero_usize() -> core::num::NonZeroUsize; + | ----------------------------------------------- `non_zero_usize` previously declared here +... +LL | fn non_zero_usize() -> usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> std::num::NonZeroUsize` + found `unsafe extern "C" fn() -> usize` + +warning: `non_null_ptr` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:255:13 + | +LL | fn non_null_ptr() -> core::ptr::NonNull; + | ----------------------------------------------- `non_null_ptr` previously declared here +... +LL | fn non_null_ptr() -> *const usize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> std::ptr::NonNull` + found `unsafe extern "C" fn() -> *const usize` + +warning: `option_non_zero_usize_incorrect` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:281:13 + | +LL | fn option_non_zero_usize_incorrect() -> usize; + | ---------------------------------------------- `option_non_zero_usize_incorrect` previously declared here +... +LL | fn option_non_zero_usize_incorrect() -> isize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> usize` + found `unsafe extern "C" fn() -> isize` + +warning: `option_non_null_ptr_incorrect` redeclared with a different signature + --> $DIR/clashing-extern-fn.rs:283:13 + | +LL | fn option_non_null_ptr_incorrect() -> *const usize; + | --------------------------------------------------- `option_non_null_ptr_incorrect` previously declared here +... +LL | fn option_non_null_ptr_incorrect() -> *const isize; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this signature doesn't match the previous declaration + | + = note: expected `unsafe extern "C" fn() -> *const usize` + found `unsafe extern "C" fn() -> *const isize` + +warning: 15 warnings emitted