Simplify clashing_extern_declarations.

compiler/rustc_lint/src/foreign_modules.rs

@@ -1,6 +1,7 @@
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
+use rustc_hir::def::DefKind;
 use rustc_middle::query::Providers;
 use rustc_middle::ty::layout::LayoutError;
 use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
@@ -22,7 +23,7 @@ pub(crate) fn get_lints() -> LintArray {
 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));
+        lint.check_foreign_item(tcx, id);
     }
 }
 
@@ -101,7 +102,7 @@ impl ClashingExternDeclarations {
 
     /// 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<hir::OwnerId> {
+    fn insert(&mut self, tcx: TyCtxt<'_>, fi: hir::ForeignItemId) -> Option<hir::OwnerId> {
         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);
@@ -115,43 +116,112 @@ impl ClashingExternDeclarations {
         }
     }
 
-    /// Get the name of the symbol that's linked against for a given extern declaration. That is,
+    #[instrument(level = "trace", skip(self, tcx))]
-    /// the name specified in a #[link_name = ...] attribute if one was specified, else, just the
+    fn check_foreign_item<'tcx>(&mut self, tcx: TyCtxt<'tcx>, this_fi: hir::ForeignItemId) {
-    /// symbol's name.
+        let DefKind::Fn = tcx.def_kind(this_fi.owner_id) else { return };
-    fn name_of_extern_decl(tcx: TyCtxt<'_>, fi: &hir::ForeignItem<'_>) -> SymbolName {
+        let Some(existing_did) = self.insert(tcx, this_fi) else { return };
+
+        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 !structurally_same_type(
+            tcx,
+            tcx.param_env(this_fi.owner_id),
+            existing_decl_ty,
+            this_decl_ty,
+            types::CItemKind::Declaration,
+        ) {
+            let orig = name_of_extern_decl(tcx, existing_did);
+
+            // Finally, emit the diagnostic.
+            let this = tcx.item_name(this_fi.owner_id.to_def_id());
+            let orig = orig.get_name();
+            let previous_decl_label = get_relevant_span(tcx, existing_did);
+            let mismatch_label = get_relevant_span(tcx, this_fi.owner_id);
+            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(),
+                mismatch_label,
+                decorator,
+            );
+        }
+    }
+}
+
+/// 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::OwnerId) -> SymbolName {
     if let Some((overridden_link_name, overridden_link_name_span)) =
-            tcx.codegen_fn_attrs(fi.owner_id).link_name.map(|overridden_link_name| {
+        tcx.codegen_fn_attrs(fi).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)
+            (overridden_link_name, tcx.get_attr(fi, sym::link_name).unwrap().span)
         })
     {
         SymbolName::Link(overridden_link_name, overridden_link_name_span)
     } else {
-            SymbolName::Normal(fi.ident.name)
+        SymbolName::Normal(tcx.item_name(fi.to_def_id()))
-        }
     }
+}
 
-    /// Checks whether two types are structurally the same enough that the declarations shouldn't
+/// We want to ensure that we use spans for both decls that include where the
-    /// clash. We need this so we don't emit a lint when two modules both declare an extern struct,
+/// name was defined, whether that was from the link_name attribute or not.
-    /// with the same members (as the declarations shouldn't clash).
+fn get_relevant_span(tcx: TyCtxt<'_>, fi: hir::OwnerId) -> Span {
-    fn structurally_same_type<'tcx>(
+    match name_of_extern_decl(tcx, fi) {
+        SymbolName::Normal(_) => tcx.def_span(fi),
+        SymbolName::Link(_, annot_span) => annot_span,
+    }
+}
+
+/// 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 {
+) -> bool {
-        fn structurally_same_type_impl<'tcx>(
+    let mut seen_types = FxHashSet::default();
+    structurally_same_type_impl(&mut seen_types, tcx, param_env, a, b, ckind)
+}
+
+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 {
+) -> bool {
     debug!("structurally_same_type_impl(tcx, a = {:?}, b = {:?})", a, b);
 
     // Given a transparent newtype, reach through and grab the inner
@@ -211,9 +281,8 @@ impl ClashingExternDeclarations {
         };
 
         #[allow(rustc::usage_of_ty_tykind)]
-                let is_primitive_or_pointer = |kind: &ty::TyKind<'_>| {
+        let is_primitive_or_pointer =
-                    kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..))
+            |kind: &ty::TyKind<'_>| kind.is_primitive() || matches!(kind, RawPtr(..) | Ref(..));
-                };
 
         ensure_sufficient_stack(|| {
             match (a_kind, b_kind) {
@@ -232,8 +301,7 @@ impl ClashingExternDeclarations {
                     // Perform a structural comparison for each field.
                     a_fields.eq_by(
                         b_fields,
-                                |&ty::FieldDef { did: a_did, .. },
+                        |&ty::FieldDef { did: a_did, .. }, &ty::FieldDef { did: b_did, .. }| {
-                                 &ty::FieldDef { did: b_did, .. }| {
                             structurally_same_type_impl(
                                 seen_types,
                                 tcx,
@@ -252,9 +320,9 @@ impl ClashingExternDeclarations {
                             seen_types, tcx, param_env, *a_ty, *b_ty, ckind,
                         )
                 }
-                        (Slice(a_ty), Slice(b_ty)) => structurally_same_type_impl(
+                (Slice(a_ty), Slice(b_ty)) => {
-                            seen_types, tcx, param_env, *a_ty, *b_ty, ckind,
+                    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(
@@ -280,9 +348,7 @@ impl ClashingExternDeclarations {
                     (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(
+                            structurally_same_type_impl(seen_types, tcx, param_env, *a, *b, ckind)
-                                        seen_types, tcx, param_env, *a, *b, ckind,
-                                    )
                         })
                         && structurally_same_type_impl(
                             seen_types,
@@ -295,9 +361,7 @@ impl ClashingExternDeclarations {
                 }
                 (Tuple(a_args), Tuple(b_args)) => {
                     a_args.iter().eq_by(b_args.iter(), |a_ty, b_ty| {
-                                structurally_same_type_impl(
+                        structurally_same_type_impl(seen_types, tcx, param_env, a_ty, b_ty, ckind)
-                                    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.
@@ -335,75 +399,4 @@ impl ClashingExternDeclarations {
             }
         })
     }
-        }
-        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,
-                    );
-                }
-            }
-        }
-    }
 }