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

@@ -293,6 +293,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

@@ -382,7 +382,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,14 @@ 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

@@ -1592,7 +1592,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(
@@ -1602,6 +1602,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,7 @@ 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 +767,12 @@ 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

@@ -760,9 +760,6 @@ passes_transparent_incompatible =
 passes_undefined_naked_function_abi =
     Rust ABI is unsupported in naked functions
 
-passes_unknown_external_lang_item =
-    unknown external lang item: `{$lang_item}`
-
 passes_unknown_feature =
     unknown feature `{$feature}`
 

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,59 @@ 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 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, (EIIDecl, CrateNum, FxIndexMap<DefId, (EIIImpl, CrateNum)>)>::default();
+        FxIndexMap::<DefId, FoundEii>::default();
 
     // println!("current crate: {}", tcx.crate_name(LOCAL_CRATE));
 
@@ -66,28 +87,28 @@ 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 +136,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 (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;
+                }
 
-            let extern_item_did = decl.eii_extern_item;
+                (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;
+            }
+        };
 
-            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();
+        let feed = tcx.create_def(
+            CRATE_DEF_ID,
+            Some(Symbol::intern(&format!("EII shim for {decl_did:?}"))),
+            DefKind::Fn,
+        );
 
-            // println!("generating {extern_item_did:?} for impl {chosen_impl:?} in crate {} with did {decl_did:?}", tcx.crate_name(LOCAL_CRATE));
+        let extern_item_did = decl.eii_extern_item;
 
-            let shim_did = feed.def_id();
+        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!("shim: {shim_did:?}");
+        // println!("generating {extern_item_did:?} for impl {chosen_impl:?} in crate {} with did {decl_did:?}", tcx.crate_name(LOCAL_CRATE));
 
-            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 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)