make linkage be external more often

Author: jdonszelmann

compiler/rustc_error_codes/src/error_codes/E0264.md

@@ -1,3 +1,5 @@
+#### this error code is no longer emitted by the compiler.
+
 An unknown external lang item was used.
 
 Erroneous code example:

compiler/rustc_hir/src/lang_items.rs

@@ -296,6 +296,10 @@ language_item_table! {
     PanicMisalignedPointerDereference, sym::panic_misaligned_pointer_dereference, panic_misaligned_pointer_dereference_fn, Target::Fn, GenericRequirement::Exact(0);
     PanicInfo,               sym::panic_info,          panic_info,                 Target::Struct,         GenericRequirement::None;
     PanicLocation,           sym::panic_location,      panic_location,             Target::Struct,         GenericRequirement::None;
+    /// Note: used to mark an extern item but now marks an externally implementable item. This means
+    /// that the PanicImpl used to be marked to be specially treated in the compiler, while it now
+    /// is only marked so we can check if it exists. There's no other reason for this lang item
+    /// anymore.
     PanicImpl,               sym::panic_impl,          panic_impl,                 Target::Fn,             GenericRequirement::None;
     PanicCannotUnwind,       sym::panic_cannot_unwind, panic_cannot_unwind,        Target::Fn,             GenericRequirement::Exact(0);
     PanicInCleanup,          sym::panic_in_cleanup,    panic_in_cleanup,           Target::Fn,             GenericRequirement::Exact(0);

compiler/rustc_middle/src/middle/eii.rs

@@ -7,6 +7,7 @@ use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 pub struct EiiMapping {
     pub extern_item: DefId,
     pub chosen_impl: DefId,
+    pub weak_linkage: bool,
 }
 
 pub type EiiMap = FxIndexMap<LocalDefId, EiiMapping>;

compiler/rustc_middle/src/mir/mod.rs

@@ -1672,7 +1672,7 @@ mod size_asserts {
     // tidy-alphabetical-start
     static_assert_size!(BasicBlockData<'_>, 128);
     static_assert_size!(LocalDecl<'_>, 40);
-    static_assert_size!(SourceScopeData<'_>, 64);
+    static_assert_size!(SourceScopeData<'_>, 72);
     static_assert_size!(Statement<'_>, 32);
     static_assert_size!(Terminator<'_>, 96);
     static_assert_size!(VarDebugInfo<'_>, 88);

compiler/rustc_middle/src/mir/mono.rs

@@ -383,7 +383,7 @@ pub struct MonoItemData {
 /// Specifies the linkage type for a `MonoItem`.
 ///
 /// See <https://llvm.org/docs/LangRef.html#linkage-types> for more details about these variants.
-#[derive(Copy, Clone, PartialEq, Debug, TyEncodable, TyDecodable, HashStable)]
+#[derive(Copy, Clone, PartialEq, Debug, TyEncodable, TyDecodable, HashStable, Eq, Hash)]
 pub enum Linkage {
     External,
     AvailableExternally,

compiler/rustc_middle/src/mir/terminator.rs

@@ -215,6 +215,12 @@ impl<O> AssertKind<O> {
         }
     }
 
+    /// Generally do not use this and use `panic_function` instead.
+    /// Gives the lang item that is required to exist for this assertion
+    /// to be emitted. This sometimes causes the assertion not to be emitted
+    /// if a lang item isn't there.
+    pub fn required_lang_item(&self) {}
+
     /// Format the message arguments for the `assert(cond, msg..)` terminator in MIR printing.
     ///
     /// Needs to be kept in sync with the run-time behavior (which is defined by

compiler/rustc_middle/src/mir/visit.rs

@@ -355,7 +355,7 @@ macro_rules! make_mir_visitor {
                         }
                         | ty::InstanceKind::AsyncDropGlueCtorShim(_def_id, None)
                         | ty::InstanceKind::DropGlue(_def_id, None)
-                        | ty::InstanceKind::EiiShim { def_id: _def_id, extern_item: _, chosen_impl: _ } => {}
+                        | ty::InstanceKind::EiiShim { def_id: _def_id, extern_item: _, chosen_impl: _, weak_linkage: _ } => {}
 
                         ty::InstanceKind::FnPtrShim(_def_id, ty)
                         | ty::InstanceKind::DropGlue(_def_id, Some(ty))

compiler/rustc_middle/src/ty/instance.rs

@@ -178,7 +178,7 @@ pub enum InstanceKind<'tcx> {
     /// Generated by externally implementable items. This function adds indirection so we can choose
     /// in the final crate whether to call an explicit implementation or, if none are given, call the
     /// default.
-    EiiShim { def_id: DefId, extern_item: DefId, chosen_impl: DefId },
+    EiiShim { def_id: DefId, extern_item: DefId, chosen_impl: DefId, weak_linkage: bool },
 }
 
 impl<'tcx> Instance<'tcx> {
@@ -416,7 +416,10 @@ pub fn fmt_instance(
         InstanceKind::FnPtrAddrShim(_, ty) => write!(f, " - shim({ty})"),
         InstanceKind::AsyncDropGlueCtorShim(_, None) => write!(f, " - shim(None)"),
         InstanceKind::AsyncDropGlueCtorShim(_, Some(ty)) => write!(f, " - shim(Some({ty}))"),
-        InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl } => {
+        InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl, weak_linkage: true } => {
+            write!(f, " - shim(eii: {extern_item:?} -> {chosen_impl:?} [weak]")
+        }
+        InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl, weak_linkage: false } => {
             write!(f, " - shim(eii: {extern_item:?} -> {chosen_impl:?})")
         }
     }

compiler/rustc_mir_transform/src/shim.rs

@@ -78,7 +78,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceKind<'tcx>) -> Body<
             receiver_by_ref,
         } => build_construct_coroutine_by_move_shim(tcx, coroutine_closure_def_id, receiver_by_ref),
 
-        e @ ty::InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl } => {
+        e @ ty::InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl, weak_linkage: _ } => {
             let source = MirSource::from_instance(e);
 
             // get the signature for the new function this shim is creating

compiler/rustc_monomorphize/src/collector.rs

@@ -1594,7 +1594,7 @@ impl<'v> RootCollector<'_, 'v> {
     /// For each externally implementable item, we should generate an alias MonoItem that
     /// determines what implementation is called. This could be a default implementation.
     fn push_extra_eii_roots(&mut self) {
-        for (shim_did, &EiiMapping { extern_item, chosen_impl, .. }) in
+        for (shim_did, &EiiMapping { extern_item, chosen_impl, weak_linkage, .. }) in
             self.tcx.get_externally_implementable_item_impls(())
         {
             self.output.push(create_fn_mono_item(
@@ -1604,6 +1604,7 @@ impl<'v> RootCollector<'_, 'v> {
                         def_id: (*shim_did).into(),
                         extern_item,
                         chosen_impl,
+                        weak_linkage,
                     },
                     args: ty::GenericArgs::empty(),
                 },

compiler/rustc_monomorphize/src/partitioning.rs

@@ -634,7 +634,12 @@ fn characteristic_def_id_of_mono_item<'tcx>(
                 ty::InstanceKind::Item(def) => def,
                 // EII shims have a characteristic defid.
                 // But it's not their own, its the one of the extern item it is implementing.
-                ty::InstanceKind::EiiShim { def_id: _, extern_item, chosen_impl: _ } => extern_item,
+                ty::InstanceKind::EiiShim {
+                    def_id: _,
+                    extern_item,
+                    chosen_impl: _,
+                    weak_linkage: _,
+                } => extern_item,
                 ty::InstanceKind::VTableShim(..)
                 | ty::InstanceKind::ReifyShim(..)
                 | ty::InstanceKind::FnPtrShim(..)
@@ -767,8 +772,10 @@ fn mono_item_linkage_and_visibility<'tcx>(
     // together anyway. LLVM ensures the last one is the one that's chosen
     // TODO: this isn't the problem if they both decided to choose either the default or the
     // same explicit impl but if their view on it differs it is a problem!
-    if let MonoItem::Fn(Instance { def: InstanceKind::EiiShim { .. }, .. }) = mono_item {
-        (Linkage::WeakAny, vis)
+    if let MonoItem::Fn(Instance { def: InstanceKind::EiiShim { weak_linkage, .. }, .. }) =
+        mono_item
+    {
+        if *weak_linkage { (Linkage::WeakAny, vis) } else { (Linkage::External, vis) }
     } else {
         (Linkage::External, vis)
     }

compiler/rustc_passes/messages.ftl

@@ -761,6 +761,9 @@ passes_trait_impl_const_stable =
 passes_transparent_incompatible =
     transparent {$target} cannot have other repr hints
 
+passes_undefined_naked_function_abi =
+    Rust ABI is unsupported in naked functions
+
 passes_unknown_external_lang_item =
     unknown external lang item: `{$lang_item}`
 

compiler/rustc_passes/src/eii.rs

@@ -6,6 +6,7 @@ use rustc_attr_parsing::{EIIDecl, EIIImpl};
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{CRATE_DEF_ID, CrateNum, DefId, LOCAL_CRATE, LocalDefId};
+use rustc_middle::bug;
 use rustc_middle::middle::eii::EiiMapping;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::CrateType;
@@ -21,39 +22,52 @@ pub(crate) fn get_externally_implementable_item_impls<'tcx>(
     tcx: TyCtxt<'tcx>,
     (): (),
 ) -> &'tcx FxIndexMap<LocalDefId, EiiMapping> {
-    // We only need to check whether there are duplicate or missing EIIs if we're
-    // emitting something that's not an rlib.
-    let needs_check = tcx.crate_types().iter().any(|kind| match *kind {
-        // Executables are leafs of the crate graph and need all EIIs to be satisfied,
-        // either with defaults or explicit implementations. So they check their crate
-        // graph to make sure this is the case.
-        CrateType::Executable => true,
-        // Proc macros are leafs of their crate graph and will be run,
-        // and so need to check the EIIs of their dependencies.
-        CrateType::ProcMacro => true,
-
-        // These are a litte difficult. We don't know whether things depending on these
-        // will perform checks to see if EIIs are implemented, or duplicated, or any other
-        // of the checks performed in this function. So we must do the checks. However,
-        // this can later lead to duplicate symbols when linking them together.
-        // For this reason, we later mark EII symbols as "globally shared" and "may conflict".
-        // In other words, if two shared libraries both provide an implementation for an EII,
-        // that's fine! Just choose one... And because their mangled symbol names are the same
-        // (that's exactly the conflict we're having) we hopefully have the same exact implementation.
-        CrateType::Dylib | CrateType::Cdylib | CrateType::Staticlib => true,
-
-        // Rlibs are just a step in the crate graph.
-        // Later on we'll link it together into an executable and over there we can check for EIIs
-        CrateType::Rlib => false,
-    });
-    if !needs_check {
-        // In this case we could only have called it when checking,
-        // and not when we were actually codegenning functions so we don't need to return any real data
-        return &*tcx.arena.alloc(FxIndexMap::default());
+    #[derive(Copy, Clone)]
+    enum Case {
+        /// We need to generate all EII shims because we are generating some final target like an
+        /// executable or library (not rlib)
+        AlwaysEmit,
+        /// We need to generate all EII shims because one of our crate types is a final target like
+        /// an executable. However, we're also generating an rlib. So. If we see explicit
+        /// definitions of EIIs we can generate them with external linkage. However, if we find
+        /// defaults, they must also be emitted because some of our crate types are final targets.
+        /// And unfortunately the rlib will also contain these definitions. However, because rlibs
+        /// will later be used in final targets, which will use `AlwaysEmit`, these symbols that were
+        /// spuriously generated in rlibs will be redefined and then flagged by the linker as
+        /// duplicate definitions. So, we have to emit EII shims which are default impls (not
+        /// explicit ones) as weak symbols.
+        EmitMaybeWeak,
+        /// We don't always need to emit EIIs because we're generating an Rlib. However, if we see
+        /// an explicit implementation, we can! Because it cannot be overwritten anymore.
+        EmitExternalIfExplicit,
     }
 
-    let mut eiis =
-        FxIndexMap::<DefId, (EIIDecl, CrateNum, FxIndexMap<DefId, (EIIImpl, CrateNum)>)>::default();
+    let has_rlib = tcx.crate_types().iter().any(|i| matches!(i, CrateType::Rlib));
+    let has_target = tcx.crate_types().iter().any(|i| !matches!(i, CrateType::Rlib));
+
+    let case = match (has_rlib, has_target) {
+        (true, true) => Case::EmitMaybeWeak,
+        (true, false) => Case::EmitExternalIfExplicit,
+        (false, true) => Case::AlwaysEmit,
+        (false, false) => {
+            bug!("no targets but somehow we are running the compiler")
+        }
+    };
+
+    #[derive(Debug)]
+    struct FoundImpl {
+        imp: EIIImpl,
+        impl_crate: CrateNum,
+    }
+
+    #[derive(Debug)]
+    struct FoundEii {
+        decl: EIIDecl,
+        decl_crate: CrateNum,
+        impls: FxIndexMap<DefId, FoundImpl>,
+    }
+
+    let mut eiis = FxIndexMap::<DefId, FoundEii>::default();
 
     // println!("current crate: {}", tcx.crate_name(LOCAL_CRATE));
 
@@ -66,28 +80,36 @@ pub(crate) fn get_externally_implementable_item_impls<'tcx>(
         // update or insert the corresponding entries
         for (did, (decl, impls)) in crate_eiis {
             eiis.entry(*did)
-                .or_insert_with(|| (*decl, cnum, Default::default()))
-                .2
-                .extend(impls.into_iter().map(|(did, i)| (*did, (*i, cnum))));
+                .or_insert_with(|| FoundEii {
+                    decl: *decl,
+                    decl_crate: cnum,
+                    impls: Default::default(),
+                })
+                .impls
+                .extend(
+                    impls
+                        .into_iter()
+                        .map(|(did, i)| (*did, FoundImpl { imp: *i, impl_crate: cnum })),
+                );
         }
     }
 
     let mut final_impls = FxIndexMap::default();
 
     // now we have all eiis! For each of them, choose one we want to actually generate.
 
-    for (decl_did, (decl, decl_crate, impls)) in eiis {
+    for (decl_did, FoundEii { decl, decl_crate, impls }) in eiis {
         // println!("for decl: {decl_did:?}: {decl:?}");
         let mut default_impls = Vec::new();
         let mut explicit_impls = Vec::new();
 
-        for (impl_did, (impl_metadata, cnum)) in impls {
-            if impl_metadata.is_default {
+        for (impl_did, FoundImpl { imp, impl_crate }) in impls {
+            if imp.is_default {
                 // println!("found default impl in {}", tcx.crate_name(cnum));
-                default_impls.push((impl_did, cnum));
+                default_impls.push((impl_did, impl_crate));
             } else {
                 // println!("found impl in {}", tcx.crate_name(cnum));
-                explicit_impls.push((impl_did, cnum));
+                explicit_impls.push((impl_did, impl_crate));
             }
         }
 
@@ -115,43 +137,78 @@ pub(crate) fn get_externally_implementable_item_impls<'tcx>(
             panic!("multiple not supported right now, but this is easily possible");
         }
 
-        // println!("impls: {explicit_impls:?}");
-        // println!("default impls: {default_impls:?}");
-
-        if let Some((chosen_impl, _)) = explicit_impls.first().or(default_impls.first()) {
-            let feed = tcx.create_def(
-                CRATE_DEF_ID,
-                Some(Symbol::intern(&format!("EII shim for {decl_did:?}"))),
-                DefKind::Fn,
-            );
-
-            let extern_item_did = decl.eii_extern_item;
-
-            feed.generics_of(tcx.generics_of(extern_item_did).clone());
-            feed.type_of(tcx.type_of(extern_item_did).clone());
-            feed.def_span(tcx.def_span(chosen_impl));
-            feed.visibility(tcx.visibility(chosen_impl));
-            feed.feed_hir();
-
-            // println!("generating {extern_item_did:?} for impl {chosen_impl:?} in crate {} with did {decl_did:?}", tcx.crate_name(LOCAL_CRATE));
-
-            let shim_did = feed.def_id();
-
-            // println!("shim: {shim_did:?}");
-
-            final_impls.insert(
-                shim_did,
-                EiiMapping { extern_item: extern_item_did, chosen_impl: *chosen_impl },
-            );
-        } else {
-            tcx.dcx().emit_err(EiiWithoutImpl {
-                current_crate_name: tcx.crate_name(LOCAL_CRATE),
-                decl_crate_name: tcx.crate_name(decl_crate),
-                name: tcx.item_name(decl_did),
-                span: decl.span,
-                help: (),
-            });
-        }
+        let (chosen_impl, weak_linkage) =
+            match (case, explicit_impls.first(), default_impls.first()) {
+                (Case::EmitExternalIfExplicit, Some((explicit, impl_crate)), _) => {
+                    if impl_crate != &LOCAL_CRATE {
+                        continue;
+                    }
+                    (explicit, false)
+                }
+                // we don't care in this case if we find no implementation yet. Another can come
+                // downstream.
+                (Case::EmitExternalIfExplicit, None, _) => {
+                    continue;
+                }
+
+                (Case::AlwaysEmit, Some((explicit, impl_crate)), _) => {
+                    if impl_crate != &LOCAL_CRATE {
+                        continue;
+                    }
+
+                    (explicit, false)
+                }
+                (Case::AlwaysEmit, _, Some((deflt, _))) => (deflt, false),
+
+                (Case::EmitMaybeWeak, Some((explicit, impl_crate)), _) => {
+                    if impl_crate != &LOCAL_CRATE {
+                        continue;
+                    }
+
+                    (explicit, false)
+                }
+                // IMPORTANT! weak linkage because the symbol will also end up in the rlib and may need
+                // to be overwritten :(
+                (Case::EmitMaybeWeak, _, Some((deflt, _))) => (deflt, true),
+
+                // We have a target to generate, but no impl to put in it. error!
+                (Case::EmitMaybeWeak | Case::AlwaysEmit, None, None) => {
+                    tcx.dcx().emit_err(EiiWithoutImpl {
+                        current_crate_name: tcx.crate_name(LOCAL_CRATE),
+                        decl_crate_name: tcx.crate_name(decl_crate),
+                        name: tcx.item_name(decl_did),
+                        span: decl.span,
+                        help: (),
+                    });
+
+                    continue;
+                }
+            };
+
+        let feed = tcx.create_def(
+            CRATE_DEF_ID,
+            Some(Symbol::intern(&format!("EII shim for {decl_did:?}"))),
+            DefKind::Fn,
+        );
+
+        let extern_item_did = decl.eii_extern_item;
+
+        feed.generics_of(tcx.generics_of(extern_item_did).clone());
+        feed.type_of(tcx.type_of(extern_item_did).clone());
+        feed.def_span(tcx.def_span(chosen_impl));
+        feed.visibility(tcx.visibility(chosen_impl));
+        feed.feed_hir();
+
+        // println!("generating {extern_item_did:?} for impl {chosen_impl:?} in crate {} with did {decl_did:?}", tcx.crate_name(LOCAL_CRATE));
+
+        let shim_did = feed.def_id();
+
+        // println!("shim: {shim_did:?}");
+
+        final_impls.insert(
+            shim_did,
+            EiiMapping { extern_item: extern_item_did, chosen_impl: *chosen_impl, weak_linkage },
+        );
     }
 
     tcx.arena.alloc(final_impls)

compiler/rustc_passes/src/errors.rs

@@ -854,14 +854,6 @@ pub(crate) struct DeprecatedAnnotationHasNoEffect {
     pub span: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag(passes_unknown_external_lang_item, code = E0264)]
-pub(crate) struct UnknownExternLangItem {
-    #[primary_span]
-    pub span: Span,
-    pub lang_item: Symbol,
-}
-
 #[derive(Diagnostic)]
 #[diag(passes_panic_unwind_without_std)]
 #[help]

compiler/rustc_passes/src/lang_items.rs

@@ -74,6 +74,10 @@ impl<'ast, 'tcx> LanguageItemCollector<'ast, 'tcx> {
                         actual_target,
                     );
                 }
+                // Exception: for EIIs the macro gets copied to both a generated macro *and* the
+                // generated extern item. We need to ignore one of these, and it must be the
+                // macrodef.
+                Some(LangItem::PanicImpl) if actual_target == Target::MacroDef => return,
                 // Known lang item with attribute on incorrect target.
                 Some(lang_item) => {
                     self.tcx.dcx().emit_err(LangItemOnIncorrectTarget {

compiler/rustc_passes/src/weak_lang_items.rs

@@ -9,7 +9,7 @@ use rustc_middle::middle::lang_items::required;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::CrateType;
 
-use crate::errors::{MissingLangItem, PanicUnwindWithoutStd, UnknownExternLangItem};
+use crate::errors::{MissingLangItem, PanicUnwindWithoutStd};
 
 /// Checks the crate for usage of weak lang items, returning a vector of all the
 /// lang items required by this crate, but not defined yet.
@@ -31,17 +31,16 @@ pub(crate) fn check_crate(
         items.missing.push(LangItem::EhCatchTypeinfo);
     }
 
-    visit::Visitor::visit_crate(&mut WeakLangItemVisitor { tcx, items }, krate);
+    visit::Visitor::visit_crate(&mut WeakLangItemVisitor { items }, krate);
 
     verify(tcx, items);
 }
 
-struct WeakLangItemVisitor<'a, 'tcx> {
-    tcx: TyCtxt<'tcx>,
+struct WeakLangItemVisitor<'a> {
     items: &'a mut lang_items::LanguageItems,
 }
 
-impl<'ast> visit::Visitor<'ast> for WeakLangItemVisitor<'_, '_> {
+impl<'ast> visit::Visitor<'ast> for WeakLangItemVisitor<'_> {
     fn visit_foreign_item(&mut self, i: &'ast ast::ForeignItem) {
         if let Some((lang_item, _)) = lang_items::extract(&i.attrs) {
             if let Some(item) = LangItem::from_name(lang_item)
@@ -50,8 +49,6 @@ impl<'ast> visit::Visitor<'ast> for WeakLangItemVisitor<'_, '_> {
                 if self.items.get(item).is_none() {
                     self.items.missing.push(item);
                 }
-            } else {
-                self.tcx.dcx().emit_err(UnknownExternLangItem { span: i.span, lang_item });
             }
         }
     }

library/core/src/panic.rs

@@ -21,6 +21,7 @@ use crate::any::Any;
 #[stable(feature = "panic_hooks", since = "1.10.0")]
 #[cfg(not(bootstrap))]
 #[eii(panic_handler)]
+#[lang = "panic_impl"]
 pub(crate) fn panic_impl(info: &PanicInfo<'_>) -> !;
 
 #[doc(hidden)]

tests/run-make/bin-emit-no-symbols/rmake.rs

@@ -10,7 +10,6 @@ use run_make_support::{llvm_readobj, rustc};
 fn main() {
     rustc().emit("obj").input("app.rs").run();
     let out = llvm_readobj().input("app.o").arg("--symbols").run();
-    out.assert_stdout_contains("rust_begin_unwind");
     out.assert_stdout_contains("rust_eh_personality");
     out.assert_stdout_contains("__rg_oom");
 }