extract single_import_can_define_name and finalize_glob_module_binding

Author: b-naber

compiler/rustc_resolve/src/ident.rs

@@ -1,3 +1,5 @@
+use std::cell::RefMut;
+
 use Determinacy::*;
 use Namespace::*;
 use rustc_ast::{self as ast, NodeId};
@@ -13,7 +15,7 @@ use rustc_span::{Ident, Span, kw, sym};
 use tracing::{debug, instrument};
 
 use crate::errors::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
-use crate::imports::Import;
+use crate::imports::{Import, NameResolution};
 use crate::late::{ConstantHasGenerics, NoConstantGenericsReason, PathSource, Rib, RibKind};
 use crate::macros::{MacroRulesScope, sub_namespace_match};
 use crate::{
@@ -37,7 +39,7 @@ impl From<UsePrelude> for bool {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 enum Shadowing {
     Restricted,
     Unrestricted,
@@ -879,53 +881,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             .into_iter()
             .find_map(|binding| if binding == ignore_binding { None } else { binding });
 
-        if let Some(Finalize { path_span, report_private, used, root_span, .. }) = finalize {
-            let Some(binding) = binding else {
-                return Err((Determined, Weak::No));
-            };
-
-            if !self.is_accessible_from(binding.vis, parent_scope.module) {
-                if report_private {
-                    self.privacy_errors.push(PrivacyError {
-                        ident,
-                        binding,
-                        dedup_span: path_span,
-                        outermost_res: None,
-                        parent_scope: *parent_scope,
-                        single_nested: path_span != root_span,
-                    });
-                } else {
-                    return Err((Determined, Weak::No));
-                }
-            }
-
-            // Forbid expanded shadowing to avoid time travel.
-            if let Some(shadowed_glob) = resolution.shadowed_glob
-                && shadowing == Shadowing::Restricted
-                && binding.expansion != LocalExpnId::ROOT
-                && binding.res() != shadowed_glob.res()
-            {
-                self.ambiguity_errors.push(AmbiguityError {
-                    kind: AmbiguityKind::GlobVsExpanded,
-                    ident,
-                    b1: binding,
-                    b2: shadowed_glob,
-                    warning: false,
-                    misc1: AmbiguityErrorMisc::None,
-                    misc2: AmbiguityErrorMisc::None,
-                });
-            }
-
-            if shadowing == Shadowing::Unrestricted
-                && binding.expansion != LocalExpnId::ROOT
-                && let NameBindingKind::Import { import, .. } = binding.kind
-                && matches!(import.kind, ImportKind::MacroExport)
-            {
-                self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
-            }
-
-            self.record_use(ident, binding, used);
-            return Ok(binding);
+        if let Some(finalize) = finalize {
+            return self.finalize_module_binding(
+                ident,
+                binding,
+                resolution.shadowed_glob,
+                parent_scope,
+                finalize,
+                shadowing,
+            );
         }
 
         let check_usable = |this: &mut Self, binding: NameBinding<'ra>| {
@@ -944,75 +908,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
         // Check if one of single imports can still define the name,
         // if it can then our result is not determined and can be invalidated.
-        for single_import in &resolution.single_imports {
-            if ignore_import == Some(*single_import) {
-                // This branch handles a cycle in single imports.
-                //
-                // For example:
-                // ```
-                // use a::b;
-                // use b as a;
-                // ```
-                // 1. Record `use a::b` as the `ignore_import` and attempt to locate `a` in the
-                //    current module.
-                // 2. Encounter the import `use b as a`, which is a `single_import` for `a`,
-                //    and try to find `b` in the current module.
-                // 3. Re-encounter the `use a::b` import since it's a `single_import` of `b`.
-                //    This leads to entering this branch.
-                continue;
-            }
-            if !self.is_accessible_from(single_import.vis, parent_scope.module) {
-                continue;
-            }
-            if let Some(ignored) = ignore_binding
-                && let NameBindingKind::Import { import, .. } = ignored.kind
-                && import == *single_import
-            {
-                // Ignore not just the binding itself, but if it has a shadowed_glob,
-                // ignore that, too, because this loop is supposed to only process
-                // named imports.
-                continue;
-            }
-
-            let Some(module) = single_import.imported_module.get() else {
-                return Err((Undetermined, Weak::No));
-            };
-            let ImportKind::Single { source, target, target_bindings, .. } = &single_import.kind
-            else {
-                unreachable!();
-            };
-            if source != target {
-                // This branch allows the binding to be defined or updated later if the target name
-                // can hide the source.
-                if target_bindings.iter().all(|binding| binding.get().is_none()) {
-                    // None of the target bindings are available, so we can't determine
-                    // if this binding is correct or not.
-                    // See more details in #124840
-                    return Err((Undetermined, Weak::No));
-                } else if target_bindings[ns].get().is_none() && binding.is_some() {
-                    // `binding.is_some()` avoids the condition where the binding
-                    // truly doesn't exist in this namespace and should return `Err(Determined)`.
-                    return Err((Undetermined, Weak::No));
-                }
-            }
-
-            match self.resolve_ident_in_module(
-                module,
-                *source,
-                ns,
-                &single_import.parent_scope,
-                None,
-                ignore_binding,
-                ignore_import,
-            ) {
-                Err((Determined, _)) => continue,
-                Ok(binding)
-                    if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) =>
-                {
-                    continue;
-                }
-                Ok(_) | Err((Undetermined, _)) => return Err((Undetermined, Weak::No)),
-            }
+        if self.single_import_can_define_name(
+            &resolution,
+            binding,
+            ns,
+            ignore_import,
+            ignore_binding,
+            parent_scope,
+        ) {
+            return Err((Undetermined, Weak::No));
         }
 
         // So we have a resolution that's from a glob import. This resolution is determined
@@ -1101,6 +1005,129 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         Err((Determined, Weak::No))
     }
 
+    fn finalize_module_binding(
+        &mut self,
+        ident: Ident,
+        binding: Option<NameBinding<'ra>>,
+        shadowed_glob: Option<NameBinding<'ra>>,
+        parent_scope: &ParentScope<'ra>,
+        finalize: Finalize,
+        shadowing: Shadowing,
+    ) -> Result<NameBinding<'ra>, (Determinacy, Weak)> {
+        let Finalize { path_span, report_private, used, root_span, .. } = finalize;
+
+        let Some(binding) = binding else {
+            return Err((Determined, Weak::No));
+        };
+
+        if !self.is_accessible_from(binding.vis, parent_scope.module) {
+            if report_private {
+                self.privacy_errors.push(PrivacyError {
+                    ident,
+                    binding,
+                    dedup_span: path_span,
+                    outermost_res: None,
+                    parent_scope: *parent_scope,
+                    single_nested: path_span != root_span,
+                });
+            } else {
+                return Err((Determined, Weak::No));
+            }
+        }
+
+        // Forbid expanded shadowing to avoid time travel.
+        if let Some(shadowed_glob) = shadowed_glob
+            && shadowing == Shadowing::Restricted
+            && binding.expansion != LocalExpnId::ROOT
+            && binding.res() != shadowed_glob.res()
+        {
+            self.ambiguity_errors.push(AmbiguityError {
+                kind: AmbiguityKind::GlobVsExpanded,
+                ident,
+                b1: binding,
+                b2: shadowed_glob,
+                warning: false,
+                misc1: AmbiguityErrorMisc::None,
+                misc2: AmbiguityErrorMisc::None,
+            });
+        }
+
+        if shadowing == Shadowing::Unrestricted
+            && binding.expansion != LocalExpnId::ROOT
+            && let NameBindingKind::Import { import, .. } = binding.kind
+            && matches!(import.kind, ImportKind::MacroExport)
+        {
+            self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
+        }
+
+        self.record_use(ident, binding, used);
+        return Ok(binding);
+    }
+
+    // Checks if a single import can define the `Ident` corresponding to `binding`.
+    // This is used to check whether we can definitively accept a glob as a resolution.
+    fn single_import_can_define_name(
+        &mut self,
+        resolution: &RefMut<'_, NameResolution<'ra>>,
+        binding: Option<NameBinding<'ra>>,
+        ns: Namespace,
+        ignore_import: Option<Import<'ra>>,
+        ignore_binding: Option<NameBinding<'ra>>,
+        parent_scope: &ParentScope<'ra>,
+    ) -> bool {
+        for single_import in &resolution.single_imports {
+            if ignore_import == Some(*single_import) {
+                continue;
+            }
+            if !self.is_accessible_from(single_import.vis, parent_scope.module) {
+                continue;
+            }
+            if let Some(ignored) = ignore_binding
+                && let NameBindingKind::Import { import, .. } = ignored.kind
+                && import == *single_import
+            {
+                continue;
+            }
+
+            let Some(module) = single_import.imported_module.get() else {
+                return true;
+            };
+            let ImportKind::Single { source, target, target_bindings, .. } = &single_import.kind
+            else {
+                unreachable!();
+            };
+            if source != target {
+                if target_bindings.iter().all(|binding| binding.get().is_none()) {
+                    return true;
+                } else if target_bindings[ns].get().is_none() && binding.is_some() {
+                    return true;
+                }
+            }
+
+            match self.resolve_ident_in_module(
+                module,
+                *source,
+                ns,
+                &single_import.parent_scope,
+                None,
+                ignore_binding,
+                ignore_import,
+            ) {
+                Err((Determined, _)) => continue,
+                Ok(binding)
+                    if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) =>
+                {
+                    continue;
+                }
+                Ok(_) | Err((Undetermined, _)) => {
+                    return true;
+                }
+            }
+        }
+
+        false
+    }
+
     /// Validate a local resolution (from ribs).
     #[instrument(level = "debug", skip(self, all_ribs))]
     fn validate_res_from_ribs(