diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 6b3facd041c..7355136aae5 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -850,6 +850,9 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> { rustc_lint::BuiltinCombinedLateLintPass::new() }); }); + }, + { + tcx.ensure().clashing_extern_declarations(()); } ); }, diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index fe020260581..27c8d02e829 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -24,10 +24,9 @@ use crate::fluent_generated as fluent; use crate::{ errors::BuiltinEllipsisInclusiveRangePatterns, lints::{ - BuiltinAnonymousParams, BuiltinBoxPointers, BuiltinClashingExtern, - BuiltinClashingExternSub, BuiltinConstNoMangle, BuiltinDeprecatedAttrLink, - BuiltinDeprecatedAttrLinkSuggestion, BuiltinDeprecatedAttrUsed, BuiltinDerefNullptr, - BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives, + BuiltinAnonymousParams, BuiltinBoxPointers, BuiltinConstNoMangle, + BuiltinDeprecatedAttrLink, BuiltinDeprecatedAttrLinkSuggestion, BuiltinDeprecatedAttrUsed, + BuiltinDerefNullptr, BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, @@ -40,7 +39,6 @@ use crate::{ BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, SuggestChangingAssocTypes, }, - types::{transparent_newtype_field, CItemKind}, EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext, }; use hir::IsAsync; @@ -49,28 +47,26 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{FnCtxt, FnKind}; use rustc_ast::{self as ast, *}; use rustc_ast_pretty::pprust::{self, expr_to_string}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{Applicability, DecorateLint, MultiSpan}; use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, GateIssue, Stability}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID}; use rustc_hir::intravisit::FnKind as HirFnKind; -use rustc_hir::{Body, FnDecl, ForeignItemKind, GenericParamKind, Node, PatKind, PredicateOrigin}; +use rustc_hir::{Body, FnDecl, GenericParamKind, Node, PatKind, PredicateOrigin}; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::layout::{LayoutError, LayoutOf}; +use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::TypeVisitableExt; -use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef}; +use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef}; use rustc_session::config::ExpectedValues; use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason}; use rustc_span::edition::Edition; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, InnerSpan, Span}; -use rustc_target::abi::{Abi, FIRST_VARIANT}; +use rustc_target::abi::Abi; use rustc_trait_selection::infer::{InferCtxtExt, TyCtxtInferExt}; use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy}; @@ -2612,381 +2608,6 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue { } } -declare_lint! { - /// The `clashing_extern_declarations` lint detects when an `extern fn` - /// has been declared with the same name but different types. - /// - /// ### Example - /// - /// ```rust - /// mod m { - /// extern "C" { - /// fn foo(); - /// } - /// } - /// - /// extern "C" { - /// fn foo(_: u32); - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// Because two symbols of the same name cannot be resolved to two - /// different functions at link time, and one function cannot possibly - /// have two types, a clashing extern declaration is almost certainly a - /// mistake. Check to make sure that the `extern` definitions are correct - /// and equivalent, and possibly consider unifying them in one location. - /// - /// This lint does not run between crates because a project may have - /// dependencies which both rely on the same extern function, but declare - /// it in a different (but valid) way. For example, they may both declare - /// an opaque type for one or more of the arguments (which would end up - /// distinct types), or use types that are valid conversions in the - /// language the `extern fn` is defined in. In these cases, the compiler - /// can't say that the clashing declaration is incorrect. - pub CLASHING_EXTERN_DECLARATIONS, - Warn, - "detects when an extern fn has been declared with the same name but different types" -} - -pub struct ClashingExternDeclarations { - /// Map of function symbol name to the first-seen hir id for that symbol name.. If seen_decls - /// contains an entry for key K, it means a symbol with name K has been seen by this lint and - /// the symbol should be reported as a clashing declaration. - // FIXME: Technically, we could just store a &'tcx str here without issue; however, the - // `impl_lint_pass` macro doesn't currently support lints parametric over a lifetime. - seen_decls: FxHashMap, -} - -/// Differentiate between whether the name for an extern decl came from the link_name attribute or -/// just from declaration itself. This is important because we don't want to report clashes on -/// symbol name if they don't actually clash because one or the other links against a symbol with a -/// different name. -enum SymbolName { - /// The name of the symbol + the span of the annotation which introduced the link name. - Link(Symbol, Span), - /// No link name, so just the name of the symbol. - Normal(Symbol), -} - -impl SymbolName { - fn get_name(&self) -> Symbol { - match self { - SymbolName::Link(s, _) | SymbolName::Normal(s) => *s, - } - } -} - -impl ClashingExternDeclarations { - pub(crate) fn new() -> Self { - ClashingExternDeclarations { seen_decls: FxHashMap::default() } - } - - /// Insert a new foreign item into the seen set. If a symbol with the same name already exists - /// for the item, return its HirId without updating the set. - fn insert(&mut self, tcx: TyCtxt<'_>, fi: &hir::ForeignItem<'_>) -> Option { - let did = fi.owner_id.to_def_id(); - let instance = Instance::new(did, ty::List::identity_for_item(tcx, did)); - let name = Symbol::intern(tcx.symbol_name(instance).name); - if let Some(&existing_id) = self.seen_decls.get(&name) { - // Avoid updating the map with the new entry when we do find a collision. We want to - // make sure we're always pointing to the first definition as the previous declaration. - // This lets us avoid emitting "knock-on" diagnostics. - Some(existing_id) - } else { - self.seen_decls.insert(name, fi.owner_id) - } - } - - /// Get the name of the symbol that's linked against for a given extern declaration. That is, - /// the name specified in a #[link_name = ...] attribute if one was specified, else, just the - /// symbol's name. - fn name_of_extern_decl(tcx: TyCtxt<'_>, fi: &hir::ForeignItem<'_>) -> SymbolName { - if let Some((overridden_link_name, overridden_link_name_span)) = - tcx.codegen_fn_attrs(fi.owner_id).link_name.map(|overridden_link_name| { - // FIXME: Instead of searching through the attributes again to get span - // information, we could have codegen_fn_attrs also give span information back for - // where the attribute was defined. However, until this is found to be a - // bottleneck, this does just fine. - (overridden_link_name, tcx.get_attr(fi.owner_id, sym::link_name).unwrap().span) - }) - { - SymbolName::Link(overridden_link_name, overridden_link_name_span) - } else { - SymbolName::Normal(fi.ident.name) - } - } - - /// 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>, - ckind: CItemKind, - ) -> bool { - fn structurally_same_type_impl<'tcx>( - seen_types: &mut FxHashSet<(Ty<'tcx>, Ty<'tcx>)>, - cx: &LateContext<'tcx>, - a: Ty<'tcx>, - b: Ty<'tcx>, - ckind: CItemKind, - ) -> bool { - debug!("structurally_same_type_impl(cx, a = {:?}, b = {:?})", a, b); - let tcx = cx.tcx; - - // Given a transparent newtype, reach through and grab the inner - // type unless the newtype makes the type non-null. - let non_transparent_ty = |mut ty: Ty<'tcx>| -> Ty<'tcx> { - loop { - if let ty::Adt(def, args) = *ty.kind() { - let is_transparent = def.repr().transparent(); - let is_non_null = crate::types::nonnull_optimization_guaranteed(tcx, def); - debug!( - "non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}", - ty, is_transparent, is_non_null - ); - if is_transparent && !is_non_null { - debug_assert_eq!(def.variants().len(), 1); - let v = &def.variant(FIRST_VARIANT); - // continue with `ty`'s non-ZST field, - // otherwise `ty` is a ZST and we can return - if let Some(field) = transparent_newtype_field(tcx, v) { - ty = field.ty(tcx, args); - continue; - } - } - } - debug!("non_transparent_ty -> {:?}", ty); - return ty; - } - }; - - let a = non_transparent_ty(a); - let b = non_transparent_ty(b); - - if !seen_types.insert((a, b)) { - // We've encountered a cycle. There's no point going any further -- the types are - // structurally the same. - true - } else if a == b { - // All nominally-same types are structurally same, too. - true - } else { - // Do a full, depth-first comparison between the two. - use rustc_type_ir::sty::TyKind::*; - let a_kind = a.kind(); - let b_kind = b.kind(); - - let compare_layouts = |a, b| -> Result> { - debug!("compare_layouts({:?}, {:?})", a, b); - let a_layout = &cx.layout_of(a)?.layout.abi(); - let b_layout = &cx.layout_of(b)?.layout.abi(); - debug!( - "comparing layouts: {:?} == {:?} = {}", - a_layout, - b_layout, - a_layout == b_layout - ); - Ok(a_layout == b_layout) - }; - - #[allow(rustc::usage_of_ty_tykind)] - let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| { - kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..)) - }; - - ensure_sufficient_stack(|| { - match (a_kind, b_kind) { - (Adt(a_def, _), Adt(b_def, _)) => { - // We can immediately rule out these types as structurally same if - // their layouts differ. - match compare_layouts(a, b) { - Ok(false) => return false, - _ => (), // otherwise, continue onto the full, fields comparison - } - - // 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()); - - // Perform a structural comparison for each field. - a_fields.eq_by( - b_fields, - |&ty::FieldDef { did: a_did, .. }, - &ty::FieldDef { did: b_did, .. }| { - structurally_same_type_impl( - seen_types, - cx, - tcx.type_of(a_did).instantiate_identity(), - tcx.type_of(b_did).instantiate_identity(), - ckind, - ) - }, - ) - } - (Array(a_ty, a_const), Array(b_ty, b_const)) => { - // For arrays, we also check the constness of the type. - a_const.kind() == b_const.kind() - && structurally_same_type_impl(seen_types, cx, *a_ty, *b_ty, ckind) - } - (Slice(a_ty), Slice(b_ty)) => { - structurally_same_type_impl(seen_types, cx, *a_ty, *b_ty, ckind) - } - (RawPtr(a_tymut), RawPtr(b_tymut)) => { - a_tymut.mutbl == b_tymut.mutbl - && structurally_same_type_impl( - seen_types, 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 - && structurally_same_type_impl(seen_types, cx, *a_ty, *b_ty, ckind) - } - (FnDef(..), FnDef(..)) => { - let a_poly_sig = a.fn_sig(tcx); - let b_poly_sig = b.fn_sig(tcx); - - // We don't compare regions, but leaving bound regions around ICEs, so - // we erase them. - let a_sig = tcx.erase_late_bound_regions(a_poly_sig); - let b_sig = tcx.erase_late_bound_regions(b_poly_sig); - - (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| { - structurally_same_type_impl(seen_types, cx, *a, *b, ckind) - }) - && structurally_same_type_impl( - seen_types, - cx, - a_sig.output(), - b_sig.output(), - ckind, - ) - } - (Tuple(a_args), Tuple(b_args)) => { - a_args.iter().eq_by(b_args.iter(), |a_ty, b_ty| { - structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind) - }) - } - // For these, it's not quite as easy to define structural-sameness quite so easily. - // For the purposes of this lint, take the conservative approach and mark them as - // not structurally same. - (Dynamic(..), Dynamic(..)) - | (Error(..), Error(..)) - | (Closure(..), Closure(..)) - | (Generator(..), Generator(..)) - | (GeneratorWitness(..), GeneratorWitness(..)) - | (Alias(ty::Projection, ..), Alias(ty::Projection, ..)) - | (Alias(ty::Inherent, ..), Alias(ty::Inherent, ..)) - | (Alias(ty::Opaque, ..), Alias(ty::Opaque, ..)) => false, - - // These definitely should have been caught above. - (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), - - // An Adt and a primitive or pointer type. This can be FFI-safe if non-null - // enum layout optimisation is being applied. - (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).unwrap_or(false) - } - } - // 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).unwrap_or(false), - } - }) - } - } - let mut seen_types = FxHashSet::default(); - structurally_same_type_impl(&mut seen_types, cx, a, b, ckind) - } -} - -impl_lint_pass!(ClashingExternDeclarations => [CLASHING_EXTERN_DECLARATIONS]); - -impl<'tcx> LateLintPass<'tcx> for ClashingExternDeclarations { - #[instrument(level = "trace", skip(self, cx))] - fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, this_fi: &hir::ForeignItem<'_>) { - if let ForeignItemKind::Fn(..) = this_fi.kind { - let tcx = cx.tcx; - if let Some(existing_did) = self.insert(tcx, this_fi) { - let existing_decl_ty = tcx.type_of(existing_did).skip_binder(); - let this_decl_ty = tcx.type_of(this_fi.owner_id).instantiate_identity(); - debug!( - "ClashingExternDeclarations: Comparing existing {:?}: {:?} to this {:?}: {:?}", - existing_did, existing_decl_ty, this_fi.owner_id, this_decl_ty - ); - // Check that the declarations match. - if !Self::structurally_same_type( - cx, - existing_decl_ty, - this_decl_ty, - CItemKind::Declaration, - ) { - let orig_fi = tcx.hir().expect_foreign_item(existing_did); - let orig = Self::name_of_extern_decl(tcx, orig_fi); - - // We want to ensure that we use spans for both decls that include where the - // name was defined, whether that was from the link_name attribute or not. - let get_relevant_span = - |fi: &hir::ForeignItem<'_>| match Self::name_of_extern_decl(tcx, fi) { - SymbolName::Normal(_) => fi.span, - SymbolName::Link(_, annot_span) => fi.span.to(annot_span), - }; - - // Finally, emit the diagnostic. - let this = this_fi.ident.name; - let orig = orig.get_name(); - let previous_decl_label = get_relevant_span(orig_fi); - let mismatch_label = get_relevant_span(this_fi); - let sub = BuiltinClashingExternSub { - tcx, - expected: existing_decl_ty, - found: this_decl_ty, - }; - let decorator = if orig == this { - BuiltinClashingExtern::SameName { - this, - orig, - previous_decl_label, - mismatch_label, - sub, - } - } else { - BuiltinClashingExtern::DiffName { - this, - orig, - previous_decl_label, - mismatch_label, - sub, - } - }; - tcx.emit_spanned_lint( - CLASHING_EXTERN_DECLARATIONS, - this_fi.hir_id(), - get_relevant_span(this_fi), - decorator, - ); - } - } - } - } -} - declare_lint! { /// The `deref_nullptr` lint detects when an null pointer is dereferenced, /// which causes [undefined behavior]. diff --git a/compiler/rustc_lint/src/foreign_modules.rs b/compiler/rustc_lint/src/foreign_modules.rs new file mode 100644 index 00000000000..61d4f8b0ba7 --- /dev/null +++ b/compiler/rustc_lint/src/foreign_modules.rs @@ -0,0 +1,409 @@ +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::stack::ensure_sufficient_stack; +use rustc_hir as hir; +use rustc_middle::query::Providers; +use rustc_middle::ty::layout::LayoutError; +use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; +use rustc_session::lint::{lint_array, LintArray}; +use rustc_span::{sym, Span, Symbol}; +use rustc_target::abi::FIRST_VARIANT; + +use crate::lints::{BuiltinClashingExtern, BuiltinClashingExternSub}; +use crate::types; + +pub(crate) fn provide(providers: &mut Providers) { + *providers = Providers { clashing_extern_declarations, ..*providers }; +} + +pub(crate) fn get_lints() -> LintArray { + lint_array!(CLASHING_EXTERN_DECLARATIONS) +} + +fn clashing_extern_declarations(tcx: TyCtxt<'_>, (): ()) { + let mut lint = ClashingExternDeclarations::new(); + for id in tcx.hir_crate_items(()).foreign_items() { + lint.check_foreign_item(tcx, tcx.hir().foreign_item(id)); + } +} + +declare_lint! { + /// The `clashing_extern_declarations` lint detects when an `extern fn` + /// has been declared with the same name but different types. + /// + /// ### Example + /// + /// ```rust + /// mod m { + /// extern "C" { + /// fn foo(); + /// } + /// } + /// + /// extern "C" { + /// fn foo(_: u32); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Because two symbols of the same name cannot be resolved to two + /// different functions at link time, and one function cannot possibly + /// have two types, a clashing extern declaration is almost certainly a + /// mistake. Check to make sure that the `extern` definitions are correct + /// and equivalent, and possibly consider unifying them in one location. + /// + /// This lint does not run between crates because a project may have + /// dependencies which both rely on the same extern function, but declare + /// it in a different (but valid) way. For example, they may both declare + /// an opaque type for one or more of the arguments (which would end up + /// distinct types), or use types that are valid conversions in the + /// language the `extern fn` is defined in. In these cases, the compiler + /// can't say that the clashing declaration is incorrect. + pub CLASHING_EXTERN_DECLARATIONS, + Warn, + "detects when an extern fn has been declared with the same name but different types" +} + +struct ClashingExternDeclarations { + /// Map of function symbol name to the first-seen hir id for that symbol name.. If seen_decls + /// contains an entry for key K, it means a symbol with name K has been seen by this lint and + /// the symbol should be reported as a clashing declaration. + // FIXME: Technically, we could just store a &'tcx str here without issue; however, the + // `impl_lint_pass` macro doesn't currently support lints parametric over a lifetime. + seen_decls: FxHashMap, +} + +/// Differentiate between whether the name for an extern decl came from the link_name attribute or +/// just from declaration itself. This is important because we don't want to report clashes on +/// symbol name if they don't actually clash because one or the other links against a symbol with a +/// different name. +enum SymbolName { + /// The name of the symbol + the span of the annotation which introduced the link name. + Link(Symbol, Span), + /// No link name, so just the name of the symbol. + Normal(Symbol), +} + +impl SymbolName { + fn get_name(&self) -> Symbol { + match self { + SymbolName::Link(s, _) | SymbolName::Normal(s) => *s, + } + } +} + +impl ClashingExternDeclarations { + pub(crate) fn new() -> Self { + ClashingExternDeclarations { seen_decls: FxHashMap::default() } + } + + /// Insert a new foreign item into the seen set. If a symbol with the same name already exists + /// for the item, return its HirId without updating the set. + fn insert(&mut self, tcx: TyCtxt<'_>, fi: &hir::ForeignItem<'_>) -> Option { + let did = fi.owner_id.to_def_id(); + let instance = Instance::new(did, ty::List::identity_for_item(tcx, did)); + let name = Symbol::intern(tcx.symbol_name(instance).name); + if let Some(&existing_id) = self.seen_decls.get(&name) { + // Avoid updating the map with the new entry when we do find a collision. We want to + // make sure we're always pointing to the first definition as the previous declaration. + // This lets us avoid emitting "knock-on" diagnostics. + Some(existing_id) + } else { + self.seen_decls.insert(name, fi.owner_id) + } + } + + /// Get the name of the symbol that's linked against for a given extern declaration. That is, + /// the name specified in a #[link_name = ...] attribute if one was specified, else, just the + /// symbol's name. + fn name_of_extern_decl(tcx: TyCtxt<'_>, fi: &hir::ForeignItem<'_>) -> SymbolName { + if let Some((overridden_link_name, overridden_link_name_span)) = + tcx.codegen_fn_attrs(fi.owner_id).link_name.map(|overridden_link_name| { + // FIXME: Instead of searching through the attributes again to get span + // information, we could have codegen_fn_attrs also give span information back for + // where the attribute was defined. However, until this is found to be a + // bottleneck, this does just fine. + (overridden_link_name, tcx.get_attr(fi.owner_id, sym::link_name).unwrap().span) + }) + { + SymbolName::Link(overridden_link_name, overridden_link_name_span) + } else { + SymbolName::Normal(fi.ident.name) + } + } + + /// 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>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + a: Ty<'tcx>, + b: Ty<'tcx>, + ckind: types::CItemKind, + ) -> bool { + fn structurally_same_type_impl<'tcx>( + seen_types: &mut FxHashSet<(Ty<'tcx>, Ty<'tcx>)>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + a: Ty<'tcx>, + b: Ty<'tcx>, + ckind: types::CItemKind, + ) -> bool { + debug!("structurally_same_type_impl(tcx, a = {:?}, b = {:?})", a, b); + + // Given a transparent newtype, reach through and grab the inner + // type unless the newtype makes the type non-null. + let non_transparent_ty = |mut ty: Ty<'tcx>| -> Ty<'tcx> { + loop { + if let ty::Adt(def, args) = *ty.kind() { + let is_transparent = def.repr().transparent(); + let is_non_null = types::nonnull_optimization_guaranteed(tcx, def); + debug!( + "non_transparent_ty({:?}) -- type is transparent? {}, type is non-null? {}", + ty, is_transparent, is_non_null + ); + if is_transparent && !is_non_null { + debug_assert_eq!(def.variants().len(), 1); + let v = &def.variant(FIRST_VARIANT); + // continue with `ty`'s non-ZST field, + // otherwise `ty` is a ZST and we can return + if let Some(field) = types::transparent_newtype_field(tcx, v) { + ty = field.ty(tcx, args); + continue; + } + } + } + debug!("non_transparent_ty -> {:?}", ty); + return ty; + } + }; + + let a = non_transparent_ty(a); + let b = non_transparent_ty(b); + + if !seen_types.insert((a, b)) { + // We've encountered a cycle. There's no point going any further -- the types are + // structurally the same. + true + } else if a == b { + // All nominally-same types are structurally same, too. + true + } else { + // Do a full, depth-first comparison between the two. + use rustc_type_ir::sty::TyKind::*; + let a_kind = a.kind(); + let b_kind = b.kind(); + + let compare_layouts = |a, b| -> Result> { + debug!("compare_layouts({:?}, {:?})", a, b); + let a_layout = &tcx.layout_of(param_env.and(a))?.layout.abi(); + let b_layout = &tcx.layout_of(param_env.and(b))?.layout.abi(); + debug!( + "comparing layouts: {:?} == {:?} = {}", + a_layout, + b_layout, + a_layout == b_layout + ); + Ok(a_layout == b_layout) + }; + + #[allow(rustc::usage_of_ty_tykind)] + let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| { + kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..)) + }; + + ensure_sufficient_stack(|| { + match (a_kind, b_kind) { + (Adt(a_def, _), Adt(b_def, _)) => { + // We can immediately rule out these types as structurally same if + // their layouts differ. + match compare_layouts(a, b) { + Ok(false) => return false, + _ => (), // otherwise, continue onto the full, fields comparison + } + + // 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()); + + // Perform a structural comparison for each field. + a_fields.eq_by( + b_fields, + |&ty::FieldDef { did: a_did, .. }, + &ty::FieldDef { did: b_did, .. }| { + structurally_same_type_impl( + seen_types, + tcx, + param_env, + tcx.type_of(a_did).instantiate_identity(), + tcx.type_of(b_did).instantiate_identity(), + ckind, + ) + }, + ) + } + (Array(a_ty, a_const), Array(b_ty, b_const)) => { + // For arrays, we also check the constness of the type. + a_const.kind() == b_const.kind() + && structurally_same_type_impl( + seen_types, tcx, param_env, *a_ty, *b_ty, ckind, + ) + } + (Slice(a_ty), Slice(b_ty)) => structurally_same_type_impl( + seen_types, tcx, param_env, *a_ty, *b_ty, ckind, + ), + (RawPtr(a_tymut), RawPtr(b_tymut)) => { + a_tymut.mutbl == b_tymut.mutbl + && structurally_same_type_impl( + seen_types, tcx, param_env, 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 + && structurally_same_type_impl( + seen_types, tcx, param_env, *a_ty, *b_ty, ckind, + ) + } + (FnDef(..), FnDef(..)) => { + let a_poly_sig = a.fn_sig(tcx); + let b_poly_sig = b.fn_sig(tcx); + + // We don't compare regions, but leaving bound regions around ICEs, so + // we erase them. + let a_sig = tcx.erase_late_bound_regions(a_poly_sig); + let b_sig = tcx.erase_late_bound_regions(b_poly_sig); + + (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| { + structurally_same_type_impl( + seen_types, tcx, param_env, *a, *b, ckind, + ) + }) + && structurally_same_type_impl( + seen_types, + tcx, + param_env, + a_sig.output(), + b_sig.output(), + ckind, + ) + } + (Tuple(a_args), Tuple(b_args)) => { + a_args.iter().eq_by(b_args.iter(), |a_ty, b_ty| { + structurally_same_type_impl( + seen_types, tcx, param_env, a_ty, b_ty, ckind, + ) + }) + } + // For these, it's not quite as easy to define structural-sameness quite so easily. + // For the purposes of this lint, take the conservative approach and mark them as + // not structurally same. + (Dynamic(..), Dynamic(..)) + | (Error(..), Error(..)) + | (Closure(..), Closure(..)) + | (Generator(..), Generator(..)) + | (GeneratorWitness(..), GeneratorWitness(..)) + | (Alias(ty::Projection, ..), Alias(ty::Projection, ..)) + | (Alias(ty::Inherent, ..), Alias(ty::Inherent, ..)) + | (Alias(ty::Opaque, ..), Alias(ty::Opaque, ..)) => false, + + // These definitely should have been caught above. + (Bool, Bool) | (Char, Char) | (Never, Never) | (Str, Str) => unreachable!(), + + // An Adt and a primitive or pointer type. This can be FFI-safe if non-null + // enum layout optimisation is being applied. + (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) = types::repr_nullable_ptr(tcx, param_env, adt, ckind) { + ty == primitive + } else { + compare_layouts(a, b).unwrap_or(false) + } + } + // 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).unwrap_or(false), + } + }) + } + } + let mut seen_types = FxHashSet::default(); + structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind) + } + + #[instrument(level = "trace", skip(self, tcx))] + fn check_foreign_item<'tcx>(&mut self, tcx: TyCtxt<'tcx>, this_fi: &hir::ForeignItem<'_>) { + if let hir::ForeignItemKind::Fn(..) = this_fi.kind { + if let Some(existing_did) = self.insert(tcx, this_fi) { + let existing_decl_ty = tcx.type_of(existing_did).skip_binder(); + let this_decl_ty = tcx.type_of(this_fi.owner_id).instantiate_identity(); + debug!( + "ClashingExternDeclarations: Comparing existing {:?}: {:?} to this {:?}: {:?}", + existing_did, existing_decl_ty, this_fi.owner_id, this_decl_ty + ); + // Check that the declarations match. + if !Self::structurally_same_type( + tcx, + tcx.param_env(this_fi.owner_id), + existing_decl_ty, + this_decl_ty, + types::CItemKind::Declaration, + ) { + let orig_fi = tcx.hir().expect_foreign_item(existing_did); + let orig = Self::name_of_extern_decl(tcx, orig_fi); + + // We want to ensure that we use spans for both decls that include where the + // name was defined, whether that was from the link_name attribute or not. + let get_relevant_span = + |fi: &hir::ForeignItem<'_>| match Self::name_of_extern_decl(tcx, fi) { + SymbolName::Normal(_) => fi.span, + SymbolName::Link(_, annot_span) => fi.span.to(annot_span), + }; + + // Finally, emit the diagnostic. + let this = this_fi.ident.name; + let orig = orig.get_name(); + let previous_decl_label = get_relevant_span(orig_fi); + let mismatch_label = get_relevant_span(this_fi); + let sub = BuiltinClashingExternSub { + tcx, + expected: existing_decl_ty, + found: this_decl_ty, + }; + let decorator = if orig == this { + BuiltinClashingExtern::SameName { + this, + orig, + previous_decl_label, + mismatch_label, + sub, + } + } else { + BuiltinClashingExtern::DiffName { + this, + orig, + previous_decl_label, + mismatch_label, + sub, + } + }; + tcx.emit_spanned_lint( + CLASHING_EXTERN_DECLARATIONS, + this_fi.hir_id(), + get_relevant_span(this_fi), + decorator, + ); + } + } + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 80bf53ea866..2e2ec1a698e 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -59,6 +59,7 @@ mod enum_intrinsics_non_enums; mod errors; mod expect; mod for_loops_over_fallibles; +mod foreign_modules; pub mod hidden_unicode_codepoints; mod internal; mod invalid_from_utf8; @@ -140,6 +141,7 @@ fluent_messages! { "../messages.ftl" } pub fn provide(providers: &mut Providers) { levels::provide(providers); expect::provide(providers); + foreign_modules::provide(providers); *providers = Providers { lint_mod, ..*providers }; } @@ -195,8 +197,6 @@ late_lint_methods!( // FIXME: Turn the computation of types which implement Debug into a query // and change this to a module lint pass MissingDebugImplementations: MissingDebugImplementations::default(), - // Keeps a global list of foreign declarations. - ClashingExternDeclarations: ClashingExternDeclarations::new(), ] ] ); @@ -282,6 +282,7 @@ fn register_builtins(store: &mut LintStore) { store.register_lints(&BuiltinCombinedEarlyLintPass::get_lints()); store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints()); store.register_lints(&BuiltinCombinedLateLintPass::get_lints()); + store.register_lints(&foreign_modules::get_lints()); add_lint_group!( "nonstandard_style", diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 226d01b79a8..24bc16af2ee 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -815,8 +815,7 @@ pub fn transparent_newtype_field<'a, 'tcx>( } /// 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; +fn ty_is_known_nonnull<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, mode: CItemKind) -> bool { match ty.kind() { ty::FnPtr(_) => true, ty::Ref(..) => true, @@ -835,8 +834,8 @@ fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKi def.variants() .iter() - .filter_map(|variant| transparent_newtype_field(cx.tcx, variant)) - .any(|field| ty_is_known_nonnull(cx, field.ty(tcx, args), mode)) + .filter_map(|variant| transparent_newtype_field(tcx, variant)) + .any(|field| ty_is_known_nonnull(tcx, field.ty(tcx, args), mode)) } _ => false, } @@ -844,15 +843,12 @@ fn ty_is_known_nonnull<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, mode: CItemKi /// 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; +fn get_nullable_type<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option> { Some(match *ty.kind() { ty::Adt(field_def, field_args) => { let inner_field_ty = { - let mut first_non_zst_ty = field_def - .variants() - .iter() - .filter_map(|v| transparent_newtype_field(cx.tcx, v)); + let mut first_non_zst_ty = + field_def.variants().iter().filter_map(|v| transparent_newtype_field(tcx, v)); debug_assert_eq!( first_non_zst_ty.clone().count(), 1, @@ -863,7 +859,7 @@ fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option Ty::new_int(tcx, ty), ty::Uint(ty) => Ty::new_uint(tcx, ty), @@ -895,43 +891,44 @@ fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option( - cx: &LateContext<'tcx>, + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>, ckind: CItemKind, ) -> Option> { - debug!("is_repr_nullable_ptr(cx, ty = {:?})", ty); + debug!("is_repr_nullable_ptr(tcx, ty = {:?})", ty); if let ty::Adt(ty_def, args) = ty.kind() { let field_ty = match &ty_def.variants().raw[..] { [var_one, var_two] => match (&var_one.fields.raw[..], &var_two.fields.raw[..]) { - ([], [field]) | ([field], []) => field.ty(cx.tcx, args), + ([], [field]) | ([field], []) => field.ty(tcx, args), _ => return None, }, _ => return None, }; - if !ty_is_known_nonnull(cx, field_ty, ckind) { + if !ty_is_known_nonnull(tcx, 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 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(); + let compute_size_skeleton = |t| SizeSkeleton::compute(t, tcx, param_env).unwrap(); if !compute_size_skeleton(ty).same_size(compute_size_skeleton(field_ty)) { bug!("improper_ctypes: Option nonnull optimization not applied?"); } // Return the nullable type this Option-like enum can be safely represented with. - let field_ty_abi = &cx.layout_of(field_ty).unwrap().abi; + let field_ty_abi = &tcx.layout_of(param_env.and(field_ty)).unwrap().abi; if let Abi::Scalar(field_ty_scalar) = field_ty_abi { - match field_ty_scalar.valid_range(cx) { + match field_ty_scalar.valid_range(&tcx) { WrappingRange { start: 0, end } - if end == field_ty_scalar.size(&cx.tcx).unsigned_int_max() - 1 => + if end == field_ty_scalar.size(&tcx).unsigned_int_max() - 1 => { - return Some(get_nullable_type(cx, field_ty).unwrap()); + return Some(get_nullable_type(tcx, field_ty).unwrap()); } WrappingRange { start: 1, .. } => { - return Some(get_nullable_type(cx, field_ty).unwrap()); + return Some(get_nullable_type(tcx, field_ty).unwrap()); } WrappingRange { start, end } => { unreachable!("Unhandled start and end range: ({}, {})", start, end) @@ -1116,7 +1113,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() { // Special-case types like `Option`. - if repr_nullable_ptr(self.cx, ty, self.mode).is_none() { + if repr_nullable_ptr(self.cx.tcx, self.cx.param_env, ty, self.mode) + .is_none() + { return FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_enum_repr_reason, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index a02f9a9f796..35fe6ab99fb 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1596,6 +1596,11 @@ rustc_queries! { separate_provide_extern } + /// Lint against `extern fn` declarations having incompatible types. + query clashing_extern_declarations(_: ()) { + desc { "checking `extern fn` declarations are compatible" } + } + /// Identifies the entry-point (e.g., the `main` function) for a given /// crate, returning `None` if there is no entry point (such as for library crates). query entry_fn(_: ()) -> Option<(DefId, EntryFnType)> {