Auto merge of #144793 - petrochenkov:extprel3, r=davidtwco

resolve: Split extern prelude into two scopes

One scope for `extern crate` items and another for `--extern` options, with the former shadowing the latter.

If in a single scope some things can overwrite other things, especially with ad hoc restrictions like `MacroExpandedExternCrateCannotShadowExternArguments`, then it's not really a single scope.
So this PR splits `Scope::ExternPrelude` into two cleaner scopes.

This is similar to how https://github.com/rust-lang/rust/pull/144131 splits module scope into two scopes for globs and non-globs, but simpler.

4 files changed, 108 insertions, 69 deletions

diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
index 988586334fa..580fa4f5b2c 100644
--- a/compiler/rustc_resolve/src/build_reduced_graph.rs
+++ b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -971,40 +971,35 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
         let imported_binding = self.r.import(binding, import);
         if ident.name != kw::Underscore && parent == self.r.graph_root {
             let norm_ident = Macros20NormalizedIdent::new(ident);
+            // FIXME: this error is technically unnecessary now when extern prelude is split into
+            // two scopes, remove it with lang team approval.
             if let Some(entry) = self.r.extern_prelude.get(&norm_ident)
                 && expansion != LocalExpnId::ROOT
                 && orig_name.is_some()
-                && !entry.is_import()
+                && entry.item_binding.is_none()
             {
                 self.r.dcx().emit_err(
                     errors::MacroExpandedExternCrateCannotShadowExternArguments { span: item.span },
                 );
-                // `return` is intended to discard this binding because it's an
-                // unregistered ambiguity error which would result in a panic
-                // caused by inconsistency `path_res`
-                // more details: https://github.com/rust-lang/rust/pull/111761
-                return;
             }
 
             use indexmap::map::Entry;
             match self.r.extern_prelude.entry(norm_ident) {
                 Entry::Occupied(mut occupied) => {
                     let entry = occupied.get_mut();
-                    if let Some(old_binding) = entry.binding.get()
-                        && old_binding.is_import()
-                    {
+                    if entry.item_binding.is_some() {
                         let msg = format!("extern crate `{ident}` already in extern prelude");
                         self.r.tcx.dcx().span_delayed_bug(item.span, msg);
                     } else {
-                        // Binding from `extern crate` item in source code can replace
-                        // a binding from `--extern` on command line here.
-                        entry.binding.set(Some(imported_binding));
+                        entry.item_binding = Some(imported_binding);
                         entry.introduced_by_item = orig_name.is_some();
                     }
                     entry
                 }
                 Entry::Vacant(vacant) => vacant.insert(ExternPreludeEntry {
-                    binding: Cell::new(Some(imported_binding)),
+                    item_binding: Some(imported_binding),
+                    flag_binding: Cell::new(None),
+                    only_item: true,
                     introduced_by_item: true,
                 }),
             };
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
index a78cf028795..8526b233ba2 100644
--- a/compiler/rustc_resolve/src/diagnostics.rs
+++ b/compiler/rustc_resolve/src/diagnostics.rs
@@ -1096,12 +1096,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         );
                     }
                 }
-                Scope::ExternPrelude => {
+                Scope::ExternPreludeItems => {
+                    // Add idents from both item and flag scopes.
                     suggestions.extend(this.extern_prelude.keys().filter_map(|ident| {
                         let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id());
                         filter_fn(res).then_some(TypoSuggestion::typo_from_ident(ident.0, res))
                     }));
                 }
+                Scope::ExternPreludeFlags => {}
                 Scope::ToolPrelude => {
                     let res = Res::NonMacroAttr(NonMacroAttrKind::Tool);
                     suggestions.extend(
@@ -1873,14 +1875,20 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
-    fn ambiguity_diagnostics(&self, ambiguity_error: &AmbiguityError<'_>) -> AmbiguityErrorDiag {
+    fn ambiguity_diagnostics(&self, ambiguity_error: &AmbiguityError<'ra>) -> AmbiguityErrorDiag {
         let AmbiguityError { kind, ident, b1, b2, misc1, misc2, .. } = *ambiguity_error;
+        let extern_prelude_ambiguity = || {
+            self.extern_prelude.get(&Macros20NormalizedIdent::new(ident)).is_some_and(|entry| {
+                entry.item_binding == Some(b1) && entry.flag_binding.get() == Some(b2)
+            })
+        };
         let (b1, b2, misc1, misc2, swapped) = if b2.span.is_dummy() && !b1.span.is_dummy() {
             // We have to print the span-less alternative first, otherwise formatting looks bad.
             (b2, b1, misc2, misc1, true)
         } else {
             (b1, b2, misc1, misc2, false)
         };
+
         let could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
             let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
             let note_msg = format!("`{ident}` could{also} refer to {what}");
@@ -1896,7 +1904,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     "consider adding an explicit import of `{ident}` to disambiguate"
                 ))
             }
-            if b.is_extern_crate() && ident.span.at_least_rust_2018() {
+            if b.is_extern_crate() && ident.span.at_least_rust_2018() && !extern_prelude_ambiguity()
+            {
                 help_msgs.push(format!("use `::{ident}` to refer to this {thing} unambiguously"))
             }
             match misc {
diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs
index 87b3fc76c96..7ab932d9f2a 100644
--- a/compiler/rustc_resolve/src/ident.rs
+++ b/compiler/rustc_resolve/src/ident.rs
@@ -102,6 +102,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             ScopeSet::All(ns)
             | ScopeSet::ModuleAndExternPrelude(ns, _)
             | ScopeSet::Late(ns, ..) => (ns, None),
+            ScopeSet::ExternPrelude => (TypeNS, None),
             ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)),
         };
         let module = match scope_set {
@@ -111,8 +112,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             _ => parent_scope.module.nearest_item_scope(),
         };
         let module_and_extern_prelude = matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..));
+        let extern_prelude = matches!(scope_set, ScopeSet::ExternPrelude);
         let mut scope = match ns {
             _ if module_and_extern_prelude => Scope::Module(module, None),
+            _ if extern_prelude => Scope::ExternPreludeItems,
             TypeNS | ValueNS => Scope::Module(module, None),
             MacroNS => Scope::DeriveHelpers(parent_scope.expansion),
         };
@@ -143,7 +146,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 Scope::Module(..) => true,
                 Scope::MacroUsePrelude => use_prelude || rust_2015,
                 Scope::BuiltinAttrs => true,
-                Scope::ExternPrelude => use_prelude || module_and_extern_prelude,
+                Scope::ExternPreludeItems | Scope::ExternPreludeFlags => {
+                    use_prelude || module_and_extern_prelude || extern_prelude
+                }
                 Scope::ToolPrelude => use_prelude,
                 Scope::StdLibPrelude => use_prelude || ns == MacroNS,
                 Scope::BuiltinTypes => true,
@@ -182,7 +187,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 Scope::Module(..) if module_and_extern_prelude => match ns {
                     TypeNS => {
                         ctxt.adjust(ExpnId::root());
-                        Scope::ExternPrelude
+                        Scope::ExternPreludeItems
                     }
                     ValueNS | MacroNS => break,
                 },
@@ -199,7 +204,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         None => {
                             ctxt.adjust(ExpnId::root());
                             match ns {
-                                TypeNS => Scope::ExternPrelude,
+                                TypeNS => Scope::ExternPreludeItems,
                                 ValueNS => Scope::StdLibPrelude,
                                 MacroNS => Scope::MacroUsePrelude,
                             }
@@ -208,8 +213,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 }
                 Scope::MacroUsePrelude => Scope::StdLibPrelude,
                 Scope::BuiltinAttrs => break, // nowhere else to search
-                Scope::ExternPrelude if module_and_extern_prelude => break,
-                Scope::ExternPrelude => Scope::ToolPrelude,
+                Scope::ExternPreludeItems => Scope::ExternPreludeFlags,
+                Scope::ExternPreludeFlags if module_and_extern_prelude || extern_prelude => break,
+                Scope::ExternPreludeFlags => Scope::ToolPrelude,
                 Scope::ToolPrelude => Scope::StdLibPrelude,
                 Scope::StdLibPrelude => match ns {
                     TypeNS => Scope::BuiltinTypes,
@@ -413,6 +419,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             ScopeSet::All(ns)
             | ScopeSet::ModuleAndExternPrelude(ns, _)
             | ScopeSet::Late(ns, ..) => (ns, None),
+            ScopeSet::ExternPrelude => (TypeNS, None),
             ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)),
         };
 
@@ -429,6 +436,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // to detect potential ambiguities.
         let mut innermost_result: Option<(NameBinding<'_>, Flags)> = None;
         let mut determinacy = Determinacy::Determined;
+        // Shadowed bindings don't need to be marked as used or non-speculatively loaded.
+        macro finalize_scope() {
+            if innermost_result.is_none() { finalize } else { None }
+        }
 
         // Go through all the scopes and try to resolve the name.
         let break_result = self.visit_scopes(
@@ -494,7 +505,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         _ => Err(Determinacy::Determined),
                     },
                     Scope::Module(module, derive_fallback_lint_id) => {
-                        let (adjusted_parent_scope, finalize) =
+                        // FIXME: use `finalize_scope` here.
+                        let (adjusted_parent_scope, adjusted_finalize) =
                             if matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..)) {
                                 (parent_scope, finalize)
                             } else {
@@ -513,7 +525,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             } else {
                                 Shadowing::Restricted
                             },
-                            finalize,
+                            adjusted_finalize,
                             ignore_binding,
                             ignore_import,
                         );
@@ -561,14 +573,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         Some(binding) => Ok((*binding, Flags::empty())),
                         None => Err(Determinacy::Determined),
                     },
-                    Scope::ExternPrelude => {
-                        match this.reborrow().extern_prelude_get(ident, finalize.is_some()) {
+                    Scope::ExternPreludeItems => {
+                        // FIXME: use `finalize_scope` here.
+                        match this.reborrow().extern_prelude_get_item(ident, finalize.is_some()) {
                             Some(binding) => Ok((binding, Flags::empty())),
                             None => Err(Determinacy::determined(
                                 this.graph_root.unexpanded_invocations.borrow().is_empty(),
                             )),
                         }
                     }
+                    Scope::ExternPreludeFlags => {
+                        match this.extern_prelude_get_flag(ident, finalize_scope!().is_some()) {
+                            Some(binding) => Ok((binding, Flags::empty())),
+                            None => Err(Determinacy::Determined),
+                        }
+                    }
                     Scope::ToolPrelude => match this.registered_tool_bindings.get(&ident) {
                         Some(binding) => Ok((*binding, Flags::empty())),
                         None => Err(Determinacy::Determined),
@@ -599,8 +618,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             if matches!(ident.name, sym::f16)
                                 && !this.tcx.features().f16()
                                 && !ident.span.allows_unstable(sym::f16)
-                                && finalize.is_some()
-                                && innermost_result.is_none()
+                                && finalize_scope!().is_some()
                             {
                                 feature_err(
                                     this.tcx.sess,
@@ -613,8 +631,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             if matches!(ident.name, sym::f128)
                                 && !this.tcx.features().f128()
                                 && !ident.span.allows_unstable(sym::f128)
-                                && finalize.is_some()
-                                && innermost_result.is_none()
+                                && finalize_scope!().is_some()
                             {
                                 feature_err(
                                     this.tcx.sess,
@@ -819,15 +836,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 assert_eq!(shadowing, Shadowing::Unrestricted);
                 return if ns != TypeNS {
                     Err((Determined, Weak::No))
-                } else if let Some(binding) =
-                    self.reborrow().extern_prelude_get(ident, finalize.is_some())
-                {
-                    Ok(binding)
-                } else if !self.graph_root.unexpanded_invocations.borrow().is_empty() {
-                    // Macro-expanded `extern crate` items can add names to extern prelude.
-                    Err((Undetermined, Weak::No))
                 } else {
-                    Err((Determined, Weak::No))
+                    let binding = self.early_resolve_ident_in_lexical_scope(
+                        ident,
+                        ScopeSet::ExternPrelude,
+                        parent_scope,
+                        finalize,
+                        finalize.is_some(),
+                        ignore_binding,
+                        ignore_import,
+                    );
+                    return binding.map_err(|determinacy| (determinacy, Weak::No));
                 };
             }
             ModuleOrUniformRoot::CurrentScope => {
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 797f4f619e3..cc30939f5e9 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -15,6 +15,7 @@
 #![feature(arbitrary_self_types)]
 #![feature(assert_matches)]
 #![feature(box_patterns)]
+#![feature(decl_macro)]
 #![feature(if_let_guard)]
 #![feature(iter_intersperse)]
 #![feature(rustc_attrs)]
@@ -114,34 +115,46 @@ impl Determinacy {
 }
 
 /// A specific scope in which a name can be looked up.
-/// This enum is currently used only for early resolution (imports and macros),
-/// but not for late resolution yet.
 #[derive(Clone, Copy, Debug)]
 enum Scope<'ra> {
+    /// Inert attributes registered by derive macros.
     DeriveHelpers(LocalExpnId),
+    /// Inert attributes registered by derive macros, but used before they are actually declared.
+    /// This scope will exist until the compatibility lint `LEGACY_DERIVE_HELPERS`
+    /// is turned into a hard error.
     DeriveHelpersCompat,
+    /// Textual `let`-like scopes introduced by `macro_rules!` items.
     MacroRules(MacroRulesScopeRef<'ra>),
-    // The node ID is for reporting the `PROC_MACRO_DERIVE_RESOLUTION_FALLBACK`
-    // lint if it should be reported.
+    /// Names declared in the given module.
+    /// The node ID is for reporting the `PROC_MACRO_DERIVE_RESOLUTION_FALLBACK`
+    /// lint if it should be reported.
     Module(Module<'ra>, Option<NodeId>),
+    /// Names introduced by `#[macro_use]` attributes on `extern crate` items.
     MacroUsePrelude,
+    /// Built-in attributes.
     BuiltinAttrs,
-    ExternPrelude,
+    /// Extern prelude names introduced by `extern crate` items.
+    ExternPreludeItems,
+    /// Extern prelude names introduced by `--extern` flags.
+    ExternPreludeFlags,
+    /// Tool modules introduced with `#![register_tool]`.
     ToolPrelude,
+    /// Standard library prelude introduced with an internal `#[prelude_import]` import.
     StdLibPrelude,
+    /// Built-in types.
     BuiltinTypes,
 }
 
 /// Names from different contexts may want to visit different subsets of all specific scopes
 /// with different restrictions when looking up the resolution.
-/// This enum is currently used only for early resolution (imports and macros),
-/// but not for late resolution yet.
 #[derive(Clone, Copy, Debug)]
 enum ScopeSet<'ra> {
     /// All scopes with the given namespace.
     All(Namespace),
     /// A module, then extern prelude (used for mixed 2015-2018 mode in macros).
     ModuleAndExternPrelude(Namespace, Module<'ra>),
+    /// Just two extern prelude scopes.
+    ExternPrelude,
     /// All scopes with macro namespace and the given macro kind restriction.
     Macro(MacroKind),
     /// All scopes with the given namespace, used for partially performing late resolution.
@@ -1013,16 +1026,18 @@ impl<'ra> NameBindingData<'ra> {
 
 #[derive(Default, Clone)]
 struct ExternPreludeEntry<'ra> {
-    binding: Cell<Option<NameBinding<'ra>>>,
+    /// Binding from an `extern crate` item.
+    item_binding: Option<NameBinding<'ra>>,
+    /// Binding from an `--extern` flag, lazily populated on first use.
+    flag_binding: Cell<Option<NameBinding<'ra>>>,
+    /// There was no `--extern` flag introducing this name,
+    /// `flag_binding` doesn't need to be populated.
+    only_item: bool,
+    /// `item_binding` is non-redundant, happens either when `only_item` is true,
+    /// or when `extern crate` introducing `item_binding` used renaming.
     introduced_by_item: bool,
 }
 
-impl ExternPreludeEntry<'_> {
-    fn is_import(&self) -> bool {
-        self.binding.get().is_some_and(|binding| binding.is_import())
-    }
-}
-
 struct DeriveData {
     resolutions: Vec<DeriveResolution>,
     helper_attrs: Vec<(usize, Ident)>,
@@ -1889,7 +1904,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         this.get_mut().traits_in_module(module, assoc_item, &mut found_traits);
                     }
                 }
-                Scope::ExternPrelude | Scope::ToolPrelude | Scope::BuiltinTypes => {}
+                Scope::ExternPreludeItems
+                | Scope::ExternPreludeFlags
+                | Scope::ToolPrelude
+                | Scope::BuiltinTypes => {}
                 _ => unreachable!(),
             }
             None::<()>
@@ -2054,7 +2072,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             // but not introduce it, as used if they are accessed from lexical scope.
             if used == Used::Scope {
                 if let Some(entry) = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident)) {
-                    if !entry.introduced_by_item && entry.binding.get() == Some(used_binding) {
+                    if !entry.introduced_by_item && entry.item_binding == Some(used_binding) {
                         return;
                     }
                 }
@@ -2210,26 +2228,30 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
-    fn extern_prelude_get<'r>(
+    fn extern_prelude_get_item<'r>(
         mut self: CmResolver<'r, 'ra, 'tcx>,
         ident: Ident,
         finalize: bool,
     ) -> Option<NameBinding<'ra>> {
-        let mut record_use = None;
         let entry = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident));
-        let binding = entry.and_then(|entry| match entry.binding.get() {
-            Some(binding) if binding.is_import() => {
-                if finalize {
-                    record_use = Some(binding);
-                }
-                Some(binding)
+        entry.and_then(|entry| entry.item_binding).map(|binding| {
+            if finalize {
+                self.get_mut().record_use(ident, binding, Used::Scope);
             }
+            binding
+        })
+    }
+
+    fn extern_prelude_get_flag(&self, ident: Ident, finalize: bool) -> Option<NameBinding<'ra>> {
+        let entry = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident));
+        entry.and_then(|entry| match entry.flag_binding.get() {
             Some(binding) => {
                 if finalize {
                     self.cstore_mut().process_path_extern(self.tcx, ident.name, ident.span);
                 }
                 Some(binding)
             }
+            None if entry.only_item => None,
             None => {
                 let crate_id = if finalize {
                     self.cstore_mut().process_path_extern(self.tcx, ident.name, ident.span)
@@ -2241,19 +2263,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         let res = Res::Def(DefKind::Mod, crate_id.as_def_id());
                         let binding =
                             self.arenas.new_pub_res_binding(res, DUMMY_SP, LocalExpnId::ROOT);
-                        entry.binding.set(Some(binding));
+                        entry.flag_binding.set(Some(binding));
                         Some(binding)
                     }
                     None => finalize.then_some(self.dummy_binding),
                 }
             }
-        });
-
-        if let Some(binding) = record_use {
-            self.get_mut().record_use(ident, binding, Used::Scope);
-        }
-
-        binding
+        })
     }
 
     /// Rustdoc uses this to resolve doc link paths in a recoverable way. `PathResult<'a>`