From c637befd8b2b0dbf79209f95500d9aedf7c3a6a6 Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Fri, 8 Aug 2025 17:47:25 +0200 Subject: [PATCH 1/3] Resolver: Batched import resolution. Collect side effects from the current set of undetermined imports and only apply them at the end. + Bless tests --- compiler/rustc_resolve/src/imports.rs | 333 +++++++++++------- compiler/rustc_resolve/src/lib.rs | 13 +- .../crates/hir-ty/src/mir/lower/as_place.rs | 2 +- tests/ui/imports/ambiguous-14.stderr | 12 +- tests/ui/imports/ambiguous-9.rs | 4 +- tests/ui/imports/ambiguous-9.stderr | 30 +- .../ui/imports/ambiguous-panic-globvsglob.rs | 4 +- .../imports/ambiguous-panic-globvsglob.stderr | 42 +-- tests/ui/imports/duplicate.stderr | 12 +- tests/ui/imports/import-loop-2.rs | 4 +- tests/ui/imports/import-loop-2.stderr | 8 +- tests/ui/imports/import4.rs | 4 +- tests/ui/imports/import4.stderr | 8 +- .../ui/imports/same-res-ambigious.fail.stderr | 2 +- .../ui/imports/same-res-ambigious.pass.stderr | 20 ++ tests/ui/imports/same-res-ambigious.rs | 3 +- tests/ui/privacy/privacy1.stderr | 24 +- tests/ui/use/use-path-segment-kw.stderr | 28 +- 18 files changed, 321 insertions(+), 232 deletions(-) create mode 100644 tests/ui/imports/same-res-ambigious.pass.stderr diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index d9f9d1ff5a479..775360b323ab7 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -48,6 +48,28 @@ pub(crate) enum PendingDecl<'ra> { Pending, } +/// The the side effect made when resolving the bindings for an underterminate import. +enum SideEffectDecls<'ra> { + None, + /// Side effect that should be applied to the field `bindings` of `ImportKind::Single`. + /// + /// The inner `Option` is the actual side effect, it tells us whether we found a binding + /// when resolving the import in this particular namespace. + /// The outer `Option` tells us if this side effect is present. + Single { + import_decls: PerNS>>>, + }, + Glob { + imported_decls: Vec<(Decl<'ra>, BindingKey, Span /* orig_ident_span */)>, + }, +} + +/// The side effect made when resolving an undeterminate import. +struct SideEffect<'ra> { + imported_module: ModuleOrUniformRoot<'ra>, + decls: SideEffectDecls<'ra>, +} + impl<'ra> PendingDecl<'ra> { pub(crate) fn decl(self) -> Option> { match self { @@ -338,7 +360,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { && (vis == import_vis || max_vis.get().is_none_or(|max_vis| vis.is_at_least(max_vis, self.tcx))) { - max_vis.set_unchecked(Some(vis.expect_local())) + max_vis.set(Some(vis.expect_local()), self) } self.arenas.alloc_decl(DeclData { @@ -596,11 +618,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Import resolution // - // This is a fixed-point algorithm. We resolve imports until our efforts - // are stymied by an unresolved import; then we bail out of the current - // module and continue. We terminate successfully once no more imports - // remain or unsuccessfully when no forward progress in resolving imports - // is made. + // This is a batched fixed-point algorithm. Each import is resolved in + // isolation, with any side effects collected for later. + // After a full pass over the current set of `indeterminate_imports`, + // the collected side effects are committed together. The process + // repeats until either no imports remain or no further progress can + // be made. /// Resolves all imports for the crate. This method performs the fixed- /// point iteration. @@ -610,14 +633,136 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { while indeterminate_count < prev_indeterminate_count { prev_indeterminate_count = indeterminate_count; indeterminate_count = 0; + let mut side_effects = Vec::new(); self.assert_speculative = true; for import in mem::take(&mut self.indeterminate_imports) { - let import_indeterminate_count = self.cm().resolve_import(import); + let (side_effect, import_indeterminate_count) = self.cm().resolve_import(import); indeterminate_count += import_indeterminate_count; match import_indeterminate_count { 0 => self.determined_imports.push(import), _ => self.indeterminate_imports.push(import), } + if let Some(side_effect) = side_effect { + side_effects.push((import, side_effect)); + } + } + self.assert_speculative = false; + self.commit_import_resolutions(side_effects); + } + } + + fn commit_import_resolutions( + &mut self, + import_resolutions: Vec<(Import<'ra>, SideEffect<'ra>)>, + ) { + assert!( + !self.assert_speculative, + "`commit_import_resolutions` should not be called during speculative resolution" + ); + for (import, side_effect) in import_resolutions.iter() { + let SideEffect { imported_module, .. } = side_effect; + import.imported_module.set_unchecked(Some(*imported_module)); + + if import.is_glob() { + let ModuleOrUniformRoot::Module(module) = imported_module else { + self.dcx().emit_err(CannotGlobImportAllCrates { span: import.span }); + continue; + }; + if import.parent_scope.module != *module { + module.glob_importers.borrow_mut_unchecked().push(*import); + } + } + } + + for (import, side_effect) in import_resolutions { + let SideEffect { imported_module, decls: side_effect_bindings } = side_effect; + let parent = import.parent_scope.module; + + match (&import.kind, side_effect_bindings) { + ( + ImportKind::Single { target, decls, .. }, + SideEffectDecls::Single { import_decls }, + ) => { + self.per_ns(|this, ns| { + match import_decls[ns] { + Some(Some(import_decl)) => { + if import_decl.is_assoc_item() + && !this.tcx.features().import_trait_associated_functions() + { + feature_err( + this.tcx.sess, + sym::import_trait_associated_functions, + import.span, + "`use` associated items of traits is unstable", + ) + .emit(); + } + this.plant_decl_into_local_module( + IdentKey::new(*target), + target.span, + ns, + import_decl, + ); + decls[ns].set_unchecked(PendingDecl::Ready(Some(import_decl))); + } + Some(None) => { + // Don't remove underscores from `single_imports`, they were never added. + if target.name != kw::Underscore { + let key = BindingKey::new(IdentKey::new(*target), ns); + this.update_local_resolution( + parent, + key, + target.span, + false, + |_, resolution| { + resolution.single_imports.swap_remove(&import); + }, + ); + } + decls[ns].set_unchecked(PendingDecl::Ready(None)); + } + None => {} + } + }); + } + (ImportKind::Glob { id, .. }, SideEffectDecls::Glob { imported_decls }) => { + let ModuleOrUniformRoot::Module(module) = imported_module else { + continue; + }; + + if module.is_trait() && !self.tcx.features().import_trait_associated_functions() + { + feature_err( + self.tcx.sess, + sym::import_trait_associated_functions, + import.span, + "`use` associated items of traits is unstable", + ) + .emit(); + } + + for (binding, key, orig_ident_span) in imported_decls { + let import_decl = self.new_import_decl(binding, import); + let warn_ambiguity = self + .resolution(import.parent_scope.module, key) + .and_then(|r| r.binding()) + .is_some_and(|binding| binding.warn_ambiguity_recursive()); + let _ = self.try_plant_decl_into_local_module( + key.ident, + orig_ident_span, + key.ns, + import_decl, + warn_ambiguity, + ); + } + + self.record_partial_res(*id, PartialRes::new(module.res().unwrap())); + } + + (_, SideEffectDecls::None) => {} + + // Something weird happened, which shouldn't have happened. + _ => unreachable!("Mismatched import kind and side effect"), } self.assert_speculative = false; } @@ -890,12 +1035,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { /// - `0` means its resolution is determined. /// - Other values mean that indeterminate exists under certain namespaces. /// - /// Meanwhile, if resolve successful, the resolved bindings are written - /// into the module. - fn resolve_import<'r>(mut self: CmResolver<'r, 'ra, 'tcx>, import: Import<'ra>) -> usize { + /// Meanwhile, if resolution is successful, the side effect of the resolution is returned. + fn resolve_import<'r>( + mut self: CmResolver<'r, 'ra, 'tcx>, + import: Import<'ra>, + ) -> (Option>, usize) { debug!( - "(resolving import for module) resolving import `{}::...` in `{}`", + "(resolving import for module) resolving import `{}::{}` in `{}`", Segment::names_to_string(&import.module_path), + import_kind_to_string(&import.kind), module_to_string(import.parent_scope.module).unwrap_or_else(|| "???".to_string()), ); let module = if let Some(module) = import.imported_module.get() { @@ -910,23 +1058,30 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match path_res { PathResult::Module(module) => module, - PathResult::Indeterminate => return 3, - PathResult::NonModule(..) | PathResult::Failed { .. } => return 0, + PathResult::Indeterminate => return (None, 3), + PathResult::NonModule(..) | PathResult::Failed { .. } => { + return (None, 0); + } } }; - import.imported_module.set_unchecked(Some(module)); - let (source, target, bindings, type_ns_only) = match import.kind { + let (source, _, bindings, type_ns_only) = match import.kind { ImportKind::Single { source, target, ref decls, type_ns_only, .. } => { (source, target, decls, type_ns_only) } ImportKind::Glob { .. } => { - self.get_mut_unchecked().resolve_glob_import(import); - return 0; + return ( + Some(SideEffect { + imported_module: module, + decls: self.resolve_glob_import(import, module), + }), + 0, + ); } _ => unreachable!(), }; + let mut import_bindings = PerNS::default(); let mut indeterminate_count = 0; self.per_ns_cm(|mut this, ns| { if !type_ns_only || ns == TypeNS { @@ -940,56 +1095,28 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &import.parent_scope, Some(import), ); - let parent = import.parent_scope.module; - let binding = match binding_result { + let pending_binding = match binding_result { Ok(binding) => { - if binding.is_assoc_item() - && !this.tcx.features().import_trait_associated_functions() - { - feature_err( - this.tcx.sess, - sym::import_trait_associated_functions, - import.span, - "`use` associated items of traits is unstable", - ) - .emit(); - } // We need the `target`, `source` can be extracted. let import_decl = this.new_import_decl(binding, import); - this.get_mut_unchecked().plant_decl_into_local_module( - IdentKey::new(target), - target.span, - ns, - import_decl, - ); - PendingDecl::Ready(Some(import_decl)) - } - Err(Determinacy::Determined) => { - // Don't remove underscores from `single_imports`, they were never added. - if target.name != kw::Underscore { - let key = BindingKey::new(IdentKey::new(target), ns); - this.get_mut_unchecked().update_local_resolution( - parent, - key, - target.span, - false, - |_, resolution| { - resolution.single_imports.swap_remove(&import); - }, - ); - } - PendingDecl::Ready(None) + Some(Some(import_decl)) } + Err(Determinacy::Determined) => Some(None), Err(Determinacy::Undetermined) => { indeterminate_count += 1; - PendingDecl::Pending + None } }; - bindings[ns].set_unchecked(binding); + import_bindings[ns] = pending_binding; } }); - - indeterminate_count + ( + Some(SideEffect { + imported_module: module, + decls: SideEffectDecls::Single { import_decls: import_bindings }, + }), + indeterminate_count, + ) } /// Performs final import resolution, consistency checks and error reporting. @@ -1540,70 +1667,40 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { false } - fn resolve_glob_import(&mut self, import: Import<'ra>) { - // This function is only called for glob imports. - let ImportKind::Glob { id, .. } = import.kind else { unreachable!() }; - - let ModuleOrUniformRoot::Module(module) = import.imported_module.get().unwrap() else { - self.dcx().emit_err(CannotGlobImportAllCrates { span: import.span }); - return; - }; - - if module.is_trait() && !self.tcx.features().import_trait_associated_functions() { - feature_err( - self.tcx.sess, - sym::import_trait_associated_functions, - import.span, - "`use` associated items of traits is unstable", - ) - .emit(); - } - - if module == import.parent_scope.module { - return; - } - - // Add to module's glob_importers - module.glob_importers.borrow_mut_unchecked().push(import); + fn resolve_glob_import( + &self, + import: Import<'ra>, + imported_module: ModuleOrUniformRoot<'ra>, + ) -> SideEffectDecls<'ra> { + match imported_module { + ModuleOrUniformRoot::Module(module) if module != import.parent_scope.module => { + let import_bindings = self + .resolutions(module) + .borrow() + .iter() + .filter_map(|(key, resolution)| { + let res = resolution.borrow(); + let orig_ident_span = res.orig_ident_span; + let binding = res.binding()?; + let mut key = *key; + let scope = match key.ident.ctxt.update_unchecked(|ctxt| { + ctxt.reverse_glob_adjust(module.expansion, import.span) + }) { + Some(Some(def)) => self.expn_def_scope(def), + Some(None) => import.parent_scope.module, + None => return None, + }; + self.is_accessible_from(binding.vis.get(), scope) + .then(|| (binding, key, orig_ident_span)) + }) + .collect::>(); - // Ensure that `resolutions` isn't borrowed during `try_define`, - // since it might get updated via a glob cycle. - let bindings = self - .resolutions(module) - .borrow() - .iter() - .filter_map(|(key, resolution)| { - let resolution = resolution.borrow(); - resolution.binding().map(|binding| (*key, binding, resolution.orig_ident_span)) - }) - .collect::>(); - for (mut key, binding, orig_ident_span) in bindings { - let scope = - match key.ident.ctxt.update_unchecked(|ctxt| { - ctxt.reverse_glob_adjust(module.expansion, import.span) - }) { - Some(Some(def)) => self.expn_def_scope(def), - Some(None) => import.parent_scope.module, - None => continue, - }; - if self.is_accessible_from(binding.vis(), scope) { - let import_decl = self.new_import_decl(binding, import); - let warn_ambiguity = self - .resolution(import.parent_scope.module, key) - .and_then(|r| r.binding()) - .is_some_and(|binding| binding.warn_ambiguity_recursive()); - let _ = self.try_plant_decl_into_local_module( - key.ident, - orig_ident_span, - key.ns, - import_decl, - warn_ambiguity, - ); + SideEffectDecls::Glob { imported_decls: import_bindings } } - } - // Record the destination of this import - self.record_partial_res(id, PartialRes::new(module.res().unwrap())); + // Errors are reported in `commit_imports_resolutions` + _ => SideEffectDecls::None, + } } // Miscellaneous post-processing, including recording re-exports, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 66f1d6b4ad55b..6c354c24d99e3 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2665,12 +2665,6 @@ mod ref_mut { true => self.p, } } - - /// Returns a mutable reference to the inner value without checking if - /// it's in a mutable state. - pub(crate) fn get_mut_unchecked(&mut self) -> &mut T { - self.p - } } /// A wrapper around a [`Cell`] that only allows mutation based on a condition in the resolver. @@ -2708,6 +2702,13 @@ mod ref_mut { CmCell(Cell::new(value)) } + pub(crate) fn set<'ra, 'tcx>(&self, val: T, r: &Resolver<'ra, 'tcx>) { + if r.assert_speculative { + panic!() + } + self.0.set(val); + } + pub(crate) fn set_unchecked(&self, val: T) { self.0.set(val); } diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/mir/lower/as_place.rs b/src/tools/rust-analyzer/crates/hir-ty/src/mir/lower/as_place.rs index cf05ec27ac37e..415ceba22cbb3 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/mir/lower/as_place.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/mir/lower/as_place.rs @@ -2,7 +2,7 @@ use hir_def::FunctionId; use intern::sym; -use rustc_type_ir::inherent::{Region as _, Ty as _}; +use rustc_type_ir::inherent::Region as _; use super::*; use crate::{ diff --git a/tests/ui/imports/ambiguous-14.stderr b/tests/ui/imports/ambiguous-14.stderr index 6823d728c368f..5c5f5469c5c3f 100644 --- a/tests/ui/imports/ambiguous-14.stderr +++ b/tests/ui/imports/ambiguous-14.stderr @@ -8,15 +8,15 @@ LL | g::foo(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `foo` could refer to the function imported here - --> $DIR/ambiguous-14.rs:18:13 + --> $DIR/ambiguous-14.rs:13:13 | LL | pub use a::*; | ^^^^ = help: consider adding an explicit import of `foo` to disambiguate note: `foo` could also refer to the function imported here - --> $DIR/ambiguous-14.rs:19:13 + --> $DIR/ambiguous-14.rs:14:13 | -LL | pub use f::*; +LL | pub use b::*; | ^^^^ = help: consider adding an explicit import of `foo` to disambiguate = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default @@ -34,15 +34,15 @@ LL | g::foo(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `foo` could refer to the function imported here - --> $DIR/ambiguous-14.rs:18:13 + --> $DIR/ambiguous-14.rs:13:13 | LL | pub use a::*; | ^^^^ = help: consider adding an explicit import of `foo` to disambiguate note: `foo` could also refer to the function imported here - --> $DIR/ambiguous-14.rs:19:13 + --> $DIR/ambiguous-14.rs:14:13 | -LL | pub use f::*; +LL | pub use b::*; | ^^^^ = help: consider adding an explicit import of `foo` to disambiguate = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default diff --git a/tests/ui/imports/ambiguous-9.rs b/tests/ui/imports/ambiguous-9.rs index e6329b8d46acc..691226ba767b6 100644 --- a/tests/ui/imports/ambiguous-9.rs +++ b/tests/ui/imports/ambiguous-9.rs @@ -12,8 +12,8 @@ pub mod prelude { mod t { pub fn date_range() {} } - pub use self::t::*; //~ WARNING ambiguous glob re-exports - pub use super::dsl::*; + pub use self::t::*; + pub use super::dsl::*; //~ WARNING ambiguous glob re-exports } use dsl::*; diff --git a/tests/ui/imports/ambiguous-9.stderr b/tests/ui/imports/ambiguous-9.stderr index da7d2d970fdda..b036617908143 100644 --- a/tests/ui/imports/ambiguous-9.stderr +++ b/tests/ui/imports/ambiguous-9.stderr @@ -32,12 +32,12 @@ LL | use super::prelude::*; = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default warning: ambiguous glob re-exports - --> $DIR/ambiguous-9.rs:15:13 + --> $DIR/ambiguous-9.rs:16:13 | LL | pub use self::t::*; - | ^^^^^^^^^^ the name `date_range` in the value namespace is first re-exported here + | ---------- but the name `date_range` in the value namespace is also re-exported here LL | pub use super::dsl::*; - | ------------- but the name `date_range` in the value namespace is also re-exported here + | ^^^^^^^^^^^^^ the name `date_range` in the value namespace is first re-exported here warning: `date_range` is ambiguous --> $DIR/ambiguous-9.rs:23:5 @@ -49,16 +49,16 @@ LL | date_range(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `date_range` could refer to the function imported here - --> $DIR/ambiguous-9.rs:19:5 + --> $DIR/ambiguous-9.rs:16:13 | -LL | use dsl::*; - | ^^^^^^ +LL | pub use super::dsl::*; + | ^^^^^^^^^^^^^ = help: consider adding an explicit import of `date_range` to disambiguate note: `date_range` could also refer to the function imported here - --> $DIR/ambiguous-9.rs:20:5 + --> $DIR/ambiguous-9.rs:15:13 | -LL | use prelude::*; - | ^^^^^^^^^^ +LL | pub use self::t::*; + | ^^^^^^^^^^ = help: consider adding an explicit import of `date_range` to disambiguate warning: 4 warnings emitted @@ -98,16 +98,16 @@ LL | date_range(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `date_range` could refer to the function imported here - --> $DIR/ambiguous-9.rs:19:5 + --> $DIR/ambiguous-9.rs:16:13 | -LL | use dsl::*; - | ^^^^^^ +LL | pub use super::dsl::*; + | ^^^^^^^^^^^^^ = help: consider adding an explicit import of `date_range` to disambiguate note: `date_range` could also refer to the function imported here - --> $DIR/ambiguous-9.rs:20:5 + --> $DIR/ambiguous-9.rs:15:13 | -LL | use prelude::*; - | ^^^^^^^^^^ +LL | pub use self::t::*; + | ^^^^^^^^^^ = help: consider adding an explicit import of `date_range` to disambiguate = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default diff --git a/tests/ui/imports/ambiguous-panic-globvsglob.rs b/tests/ui/imports/ambiguous-panic-globvsglob.rs index 335fba74b2088..041cf133e58ed 100644 --- a/tests/ui/imports/ambiguous-panic-globvsglob.rs +++ b/tests/ui/imports/ambiguous-panic-globvsglob.rs @@ -3,7 +3,6 @@ mod m1 { pub use core::prelude::v1::*; } -//@ check-pass mod m2 { pub use std::prelude::v1::*; } @@ -18,6 +17,5 @@ fn foo() { panic!(); //~^ WARN: `panic` is ambiguous [ambiguous_panic_imports] //~| WARN: this was previously accepted by the compiler - //~| WARN: `panic` is ambiguous [ambiguous_glob_imports] - //~| WARN: this was previously accepted by the compiler + //~| ERROR: `panic` is ambiguous } diff --git a/tests/ui/imports/ambiguous-panic-globvsglob.stderr b/tests/ui/imports/ambiguous-panic-globvsglob.stderr index 8e216b21734f5..56ba2b7ac9612 100644 --- a/tests/ui/imports/ambiguous-panic-globvsglob.stderr +++ b/tests/ui/imports/ambiguous-panic-globvsglob.stderr @@ -1,28 +1,25 @@ -warning: `panic` is ambiguous - --> $DIR/ambiguous-panic-globvsglob.rs:18:5 +error[E0659]: `panic` is ambiguous + --> $DIR/ambiguous-panic-globvsglob.rs:17:5 | LL | panic!(); | ^^^^^ ambiguous name | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `panic` could refer to the macro imported here - --> $DIR/ambiguous-panic-globvsglob.rs:12:9 + --> $DIR/ambiguous-panic-globvsglob.rs:11:9 | LL | use m1::*; | ^^^^^ = help: consider adding an explicit import of `panic` to disambiguate note: `panic` could also refer to the macro imported here - --> $DIR/ambiguous-panic-globvsglob.rs:13:9 + --> $DIR/ambiguous-panic-globvsglob.rs:12:9 | LL | use m2::*; | ^^^^^ = help: consider adding an explicit import of `panic` to disambiguate - = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default warning: `panic` is ambiguous - --> $DIR/ambiguous-panic-globvsglob.rs:18:5 + --> $DIR/ambiguous-panic-globvsglob.rs:17:5 | LL | panic!(); | ^^^^^ ambiguous name @@ -31,7 +28,7 @@ LL | panic!(); = note: for more information, see issue #147319 = note: ambiguous because of a conflict between a name from a glob import and an outer scope during import or macro resolution note: `panic` could refer to the macro imported here - --> $DIR/ambiguous-panic-globvsglob.rs:12:9 + --> $DIR/ambiguous-panic-globvsglob.rs:11:9 | LL | use m1::*; | ^^^^^ @@ -40,29 +37,6 @@ note: `panic` could also refer to a macro from prelude --> $SRC_DIR/std/src/prelude/mod.rs:LL:COL = note: `#[warn(ambiguous_panic_imports)]` (part of `#[warn(future_incompatible)]`) on by default -warning: 2 warnings emitted - -Future incompatibility report: Future breakage diagnostic: -warning: `panic` is ambiguous - --> $DIR/ambiguous-panic-globvsglob.rs:18:5 - | -LL | panic!(); - | ^^^^^ ambiguous name - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #114095 - = note: ambiguous because of multiple glob imports of a name in the same module -note: `panic` could refer to the macro imported here - --> $DIR/ambiguous-panic-globvsglob.rs:12:9 - | -LL | use m1::*; - | ^^^^^ - = help: consider adding an explicit import of `panic` to disambiguate -note: `panic` could also refer to the macro imported here - --> $DIR/ambiguous-panic-globvsglob.rs:13:9 - | -LL | use m2::*; - | ^^^^^ - = help: consider adding an explicit import of `panic` to disambiguate - = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default +error: aborting due to 1 previous error; 1 warning emitted +For more information about this error, try `rustc --explain E0659`. diff --git a/tests/ui/imports/duplicate.stderr b/tests/ui/imports/duplicate.stderr index 9252a041749d5..47e76ce1c3f00 100644 --- a/tests/ui/imports/duplicate.stderr +++ b/tests/ui/imports/duplicate.stderr @@ -78,15 +78,15 @@ LL | g::foo(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `foo` could refer to the function imported here - --> $DIR/duplicate.rs:29:13 + --> $DIR/duplicate.rs:24:13 | LL | pub use crate::a::*; | ^^^^^^^^^^^ = help: consider adding an explicit import of `foo` to disambiguate note: `foo` could also refer to the function imported here - --> $DIR/duplicate.rs:30:13 + --> $DIR/duplicate.rs:25:13 | -LL | pub use crate::f::*; +LL | pub use crate::b::*; | ^^^^^^^^^^^ = help: consider adding an explicit import of `foo` to disambiguate = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default @@ -106,15 +106,15 @@ LL | g::foo(); = note: for more information, see issue #114095 = note: ambiguous because of multiple glob imports of a name in the same module note: `foo` could refer to the function imported here - --> $DIR/duplicate.rs:29:13 + --> $DIR/duplicate.rs:24:13 | LL | pub use crate::a::*; | ^^^^^^^^^^^ = help: consider adding an explicit import of `foo` to disambiguate note: `foo` could also refer to the function imported here - --> $DIR/duplicate.rs:30:13 + --> $DIR/duplicate.rs:25:13 | -LL | pub use crate::f::*; +LL | pub use crate::b::*; | ^^^^^^^^^^^ = help: consider adding an explicit import of `foo` to disambiguate = note: `#[warn(ambiguous_glob_imports)]` (part of `#[warn(future_incompatible)]`) on by default diff --git a/tests/ui/imports/import-loop-2.rs b/tests/ui/imports/import-loop-2.rs index 42f9a07fff389..af5c6509d38b6 100644 --- a/tests/ui/imports/import-loop-2.rs +++ b/tests/ui/imports/import-loop-2.rs @@ -1,9 +1,9 @@ mod a { - pub use crate::b::x; + pub use crate::b::x; //~ ERROR unresolved import `crate::b::x` } mod b { - pub use crate::a::x; //~ ERROR unresolved import `crate::a::x` + pub use crate::a::x; fn main() { let y = x; } } diff --git a/tests/ui/imports/import-loop-2.stderr b/tests/ui/imports/import-loop-2.stderr index 2ef40c4e21829..dfa7cf35eaac2 100644 --- a/tests/ui/imports/import-loop-2.stderr +++ b/tests/ui/imports/import-loop-2.stderr @@ -1,8 +1,8 @@ -error[E0432]: unresolved import `crate::a::x` - --> $DIR/import-loop-2.rs:6:13 +error[E0432]: unresolved import `crate::b::x` + --> $DIR/import-loop-2.rs:2:13 | -LL | pub use crate::a::x; - | ^^^^^^^^^^^ no `x` in `a` +LL | pub use crate::b::x; + | ^^^^^^^^^^^ no `x` in `b` error: aborting due to 1 previous error diff --git a/tests/ui/imports/import4.rs b/tests/ui/imports/import4.rs index f670cc06201c0..76a9887534c34 100644 --- a/tests/ui/imports/import4.rs +++ b/tests/ui/imports/import4.rs @@ -1,4 +1,4 @@ -mod a { pub use crate::b::foo; } -mod b { pub use crate::a::foo; } //~ ERROR unresolved import `crate::a::foo` +mod a { pub use crate::b::foo; } //~ ERROR unresolved import `crate::b::foo` +mod b { pub use crate::a::foo; } fn main() { println!("loop"); } diff --git a/tests/ui/imports/import4.stderr b/tests/ui/imports/import4.stderr index 4faa5f0520a9d..07fdc6a3d0ced 100644 --- a/tests/ui/imports/import4.stderr +++ b/tests/ui/imports/import4.stderr @@ -1,8 +1,8 @@ -error[E0432]: unresolved import `crate::a::foo` - --> $DIR/import4.rs:2:17 +error[E0432]: unresolved import `crate::b::foo` + --> $DIR/import4.rs:1:17 | -LL | mod b { pub use crate::a::foo; } - | ^^^^^^^^^^^^^ no `foo` in `a` +LL | mod a { pub use crate::b::foo; } + | ^^^^^^^^^^^^^ no `foo` in `b` error: aborting due to 1 previous error diff --git a/tests/ui/imports/same-res-ambigious.fail.stderr b/tests/ui/imports/same-res-ambigious.fail.stderr index dfd7c5a5f94e0..aeeff5f2e2ad8 100644 --- a/tests/ui/imports/same-res-ambigious.fail.stderr +++ b/tests/ui/imports/same-res-ambigious.fail.stderr @@ -1,5 +1,5 @@ error[E0603]: derive macro `Embed` is private - --> $DIR/same-res-ambigious.rs:8:28 + --> $DIR/same-res-ambigious.rs:7:28 | LL | #[derive(ambigious_extern::Embed)] | ^^^^^ private derive macro diff --git a/tests/ui/imports/same-res-ambigious.pass.stderr b/tests/ui/imports/same-res-ambigious.pass.stderr new file mode 100644 index 0000000000000..a1d75a4387f66 --- /dev/null +++ b/tests/ui/imports/same-res-ambigious.pass.stderr @@ -0,0 +1,20 @@ +error[E0603]: derive macro `Embed` is private + --> $DIR/same-res-ambigious.rs:7:28 + | +LL | #[derive(ambigious_extern::Embed)] + | ^^^^^ private derive macro + | +note: the derive macro `Embed` is defined here + --> $DIR/auxiliary/same-res-ambigious-extern.rs:11:9 + | +LL | pub use RustEmbed as Embed; + | ^^^^^^^^^ +help: import `Embed` directly + | +LL - #[derive(ambigious_extern::Embed)] +LL + #[derive(same_res_ambigious_extern_macro::RustEmbed)] + | + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0603`. diff --git a/tests/ui/imports/same-res-ambigious.rs b/tests/ui/imports/same-res-ambigious.rs index b5c13a15b7c9c..52a40450b942f 100644 --- a/tests/ui/imports/same-res-ambigious.rs +++ b/tests/ui/imports/same-res-ambigious.rs @@ -1,11 +1,10 @@ //@ edition: 2018 //@ revisions: fail pass -//@[pass] check-pass //@[pass] aux-crate: ambigious_extern=same-res-ambigious-extern.rs //@[fail] aux-crate: ambigious_extern=same-res-ambigious-extern-fail.rs // see https://github.com/rust-lang/rust/pull/147196 -#[derive(ambigious_extern::Embed)] //[fail]~ ERROR: derive macro `Embed` is private +#[derive(ambigious_extern::Embed)] //~ ERROR: derive macro `Embed` is private struct Foo{} fn main(){} diff --git a/tests/ui/privacy/privacy1.stderr b/tests/ui/privacy/privacy1.stderr index f62ef3ae2e4cc..abfc7ae917dc5 100644 --- a/tests/ui/privacy/privacy1.stderr +++ b/tests/ui/privacy/privacy1.stderr @@ -23,18 +23,6 @@ LL | mod baz { | ^^^^^^^ = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error[E0603]: module `baz` is private - --> $DIR/privacy1.rs:148:18 - | -LL | use bar::baz; - | ^^^ private module - | -note: the module `baz` is defined here - --> $DIR/privacy1.rs:57:5 - | -LL | mod baz { - | ^^^^^^^ - error[E0603]: module `i` is private --> $DIR/privacy1.rs:172:20 | @@ -47,6 +35,18 @@ note: the module `i` is defined here LL | mod i { | ^^^^^ +error[E0603]: module `baz` is private + --> $DIR/privacy1.rs:148:18 + | +LL | use bar::baz; + | ^^^ private module + | +note: the module `baz` is defined here + --> $DIR/privacy1.rs:57:5 + | +LL | mod baz { + | ^^^^^^^ + error[E0603]: module `baz` is private --> $DIR/privacy1.rs:111:21 | diff --git a/tests/ui/use/use-path-segment-kw.stderr b/tests/ui/use/use-path-segment-kw.stderr index a5cfa47df3b2a..29d3f796ec4e8 100644 --- a/tests/ui/use/use-path-segment-kw.stderr +++ b/tests/ui/use/use-path-segment-kw.stderr @@ -261,51 +261,51 @@ LL | pub use self::{self as _self6}; | + + error[E0252]: the name `crate` is defined multiple times - --> $DIR/use-path-segment-kw.rs:134:13 + --> $DIR/use-path-segment-kw.rs:202:13 | LL | use crate; | ----- previous import of the module `crate` here ... -LL | use self::crate; +LL | use crate::self; | ^^^^^^^^^^^ `crate` reimported here | = note: `crate` must be defined only once in the type namespace of this module error[E0252]: the name `crate` is defined multiple times - --> $DIR/use-path-segment-kw.rs:137:20 + --> $DIR/use-path-segment-kw.rs:206:21 | LL | use crate; | ----- previous import of the module `crate` here ... -LL | use self::{crate}; - | -----------^^^^^-- - | | | - | | `crate` reimported here +LL | use crate::{self}; + | ------------^^^^-- + | | | + | | `crate` reimported here | help: remove unnecessary import | = note: `crate` must be defined only once in the type namespace of this module error[E0252]: the name `crate` is defined multiple times - --> $DIR/use-path-segment-kw.rs:202:13 + --> $DIR/use-path-segment-kw.rs:134:13 | LL | use crate; | ----- previous import of the module `crate` here ... -LL | use crate::self; +LL | use self::crate; | ^^^^^^^^^^^ `crate` reimported here | = note: `crate` must be defined only once in the type namespace of this module error[E0252]: the name `crate` is defined multiple times - --> $DIR/use-path-segment-kw.rs:206:21 + --> $DIR/use-path-segment-kw.rs:137:20 | LL | use crate; | ----- previous import of the module `crate` here ... -LL | use crate::{self}; - | ------------^^^^-- - | | | - | | `crate` reimported here +LL | use self::{crate}; + | -----------^^^^^-- + | | | + | | `crate` reimported here | help: remove unnecessary import | = note: `crate` must be defined only once in the type namespace of this module From 9306aaba288de9f68fe224a570cd7849bfee7d79 Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Thu, 19 Feb 2026 19:03:00 +0100 Subject: [PATCH 2/3] address review --- compiler/rustc_resolve/src/imports.rs | 168 +++++++++++--------------- compiler/rustc_resolve/src/lib.rs | 34 ++++-- 2 files changed, 93 insertions(+), 109 deletions(-) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 775360b323ab7..d5bca9cebddc4 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -48,26 +48,14 @@ pub(crate) enum PendingDecl<'ra> { Pending, } -/// The the side effect made when resolving the bindings for an underterminate import. -enum SideEffectDecls<'ra> { - None, - /// Side effect that should be applied to the field `bindings` of `ImportKind::Single`. - /// - /// The inner `Option` is the actual side effect, it tells us whether we found a binding - /// when resolving the import in this particular namespace. - /// The outer `Option` tells us if this side effect is present. - Single { - import_decls: PerNS>>>, - }, - Glob { - imported_decls: Vec<(Decl<'ra>, BindingKey, Span /* orig_ident_span */)>, - }, +enum ImportResolutionKind<'ra> { + Single(PerNS>), + Glob(Vec<(Decl<'ra>, BindingKey, Span /* orig_ident_span */)>), } -/// The side effect made when resolving an undeterminate import. -struct SideEffect<'ra> { +struct ImportResolution<'ra> { + kind: ImportResolutionKind<'ra>, imported_module: ModuleOrUniformRoot<'ra>, - decls: SideEffectDecls<'ra>, } impl<'ra> PendingDecl<'ra> { @@ -412,15 +400,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // assert!(!deep_decl.is_glob_import()); if old_glob_decl.ambiguity.get().is_some() && glob_decl.ambiguity.get().is_none() { // Do not lose glob ambiguities when re-fetching the glob. - glob_decl.ambiguity.set_unchecked(old_glob_decl.ambiguity.get()); + glob_decl.ambiguity.set(old_glob_decl.ambiguity.get(), self); } if glob_decl.is_ambiguity_recursive() { - glob_decl.warn_ambiguity.set_unchecked(true); + glob_decl.warn_ambiguity.set(true, self); } glob_decl } else if glob_decl.res() != old_glob_decl.res() { - old_glob_decl.ambiguity.set_unchecked(Some(glob_decl)); - old_glob_decl.warn_ambiguity.set_unchecked(warn_ambiguity); + old_glob_decl.ambiguity.set(Some(glob_decl), self); + old_glob_decl.warn_ambiguity.set(warn_ambiguity, self); if warn_ambiguity { old_glob_decl } else { @@ -436,8 +424,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { glob_decl } else if glob_decl.is_ambiguity_recursive() && !old_glob_decl.is_ambiguity_recursive() { // Overwriting a non-ambiguous glob import with an ambiguous glob import. - old_glob_decl.ambiguity.set_unchecked(Some(glob_decl)); - old_glob_decl.warn_ambiguity.set_unchecked(true); + old_glob_decl.ambiguity.set(Some(glob_decl), self); + old_glob_decl.warn_ambiguity.set(true, self); old_glob_decl } else { old_glob_decl @@ -461,7 +449,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // because they can be fetched by glob imports from those modules, and bring traits // into scope both directly and through glob imports. let key = BindingKey::new_disambiguated(ident, ns, || { - module.underscore_disambiguator.update_unchecked(|d| d + 1); + module.underscore_disambiguator.update(self, |d| d + 1); module.underscore_disambiguator.get() }); self.update_local_resolution( @@ -528,9 +516,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Ensure that `resolution` isn't borrowed when defining in the module's glob importers, // during which the resolution might end up getting re-defined via a glob cycle. let (binding, t, warn_ambiguity) = { - let resolution = &mut *self - .resolution_or_default(module, key, orig_ident_span) - .borrow_mut_unchecked(); + let resolution = + &mut *self.resolution_or_default(module, key, orig_ident_span).borrow_mut(self); let old_decl = resolution.binding(); let t = f(self, resolution); @@ -544,7 +531,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } }; - let Ok(glob_importers) = module.glob_importers.try_borrow_mut_unchecked() else { + let Ok(glob_importers) = module.glob_importers.try_borrow_mut(self) else { return t; }; @@ -619,9 +606,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Import resolution // // This is a batched fixed-point algorithm. Each import is resolved in - // isolation, with any side effects collected for later. + // isolation, with any resolutions collected for later. // After a full pass over the current set of `indeterminate_imports`, - // the collected side effects are committed together. The process + // the collected resolutions are committed together. The process // repeats until either no imports remain or no further progress can // be made. @@ -633,59 +620,51 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { while indeterminate_count < prev_indeterminate_count { prev_indeterminate_count = indeterminate_count; indeterminate_count = 0; - let mut side_effects = Vec::new(); + let mut resolutions = Vec::new(); self.assert_speculative = true; for import in mem::take(&mut self.indeterminate_imports) { - let (side_effect, import_indeterminate_count) = self.cm().resolve_import(import); + let (resolution, import_indeterminate_count) = self.cm().resolve_import(import); indeterminate_count += import_indeterminate_count; match import_indeterminate_count { 0 => self.determined_imports.push(import), _ => self.indeterminate_imports.push(import), } - if let Some(side_effect) = side_effect { - side_effects.push((import, side_effect)); + if let Some(resolution) = resolution { + resolutions.push((import, resolution)); } } self.assert_speculative = false; - self.commit_import_resolutions(side_effects); + self.write_import_resolutions(resolutions); } } - fn commit_import_resolutions( + fn write_import_resolutions( &mut self, - import_resolutions: Vec<(Import<'ra>, SideEffect<'ra>)>, + import_resolutions: Vec<(Import<'ra>, ImportResolution<'ra>)>, ) { - assert!( - !self.assert_speculative, - "`commit_import_resolutions` should not be called during speculative resolution" - ); - for (import, side_effect) in import_resolutions.iter() { - let SideEffect { imported_module, .. } = side_effect; - import.imported_module.set_unchecked(Some(*imported_module)); + for (import, resolution) in &import_resolutions { + let ImportResolution { imported_module, .. } = resolution; + import.imported_module.set(Some(*imported_module), self); - if import.is_glob() { - let ModuleOrUniformRoot::Module(module) = imported_module else { - self.dcx().emit_err(CannotGlobImportAllCrates { span: import.span }); - continue; - }; - if import.parent_scope.module != *module { - module.glob_importers.borrow_mut_unchecked().push(*import); - } + if import.is_glob() + && let ModuleOrUniformRoot::Module(module) = imported_module + && import.parent_scope.module != *module + { + module.glob_importers.borrow_mut(self).push(*import); } } - for (import, side_effect) in import_resolutions { - let SideEffect { imported_module, decls: side_effect_bindings } = side_effect; - let parent = import.parent_scope.module; + for (import, resolution) in import_resolutions { + let ImportResolution { imported_module, kind: resolution_kind } = resolution; - match (&import.kind, side_effect_bindings) { + match (&import.kind, resolution_kind) { ( ImportKind::Single { target, decls, .. }, - SideEffectDecls::Single { import_decls }, + ImportResolutionKind::Single(import_decls), ) => { self.per_ns(|this, ns| { match import_decls[ns] { - Some(Some(import_decl)) => { + PendingDecl::Ready(Some(import_decl)) => { if import_decl.is_assoc_item() && !this.tcx.features().import_trait_associated_functions() { @@ -703,14 +682,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ns, import_decl, ); - decls[ns].set_unchecked(PendingDecl::Ready(Some(import_decl))); + decls[ns].set(PendingDecl::Ready(Some(import_decl)), this); } - Some(None) => { + PendingDecl::Ready(None) => { // Don't remove underscores from `single_imports`, they were never added. if target.name != kw::Underscore { let key = BindingKey::new(IdentKey::new(*target), ns); this.update_local_resolution( - parent, + import.parent_scope.module, key, target.span, false, @@ -719,14 +698,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }, ); } - decls[ns].set_unchecked(PendingDecl::Ready(None)); + decls[ns].set(PendingDecl::Ready(None), this); } - None => {} + PendingDecl::Pending => {} } }); } - (ImportKind::Glob { id, .. }, SideEffectDecls::Glob { imported_decls }) => { + (ImportKind::Glob { id, .. }, ImportResolutionKind::Glob(imported_decls)) => { let ModuleOrUniformRoot::Module(module) = imported_module else { + self.dcx().emit_err(CannotGlobImportAllCrates { span: import.span }); continue; }; @@ -759,12 +739,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { self.record_partial_res(*id, PartialRes::new(module.res().unwrap())); } - (_, SideEffectDecls::None) => {} - // Something weird happened, which shouldn't have happened. - _ => unreachable!("Mismatched import kind and side effect"), + _ => unreachable!("mismatched import and resolution kind"), } - self.assert_speculative = false; } } @@ -1035,11 +1012,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { /// - `0` means its resolution is determined. /// - Other values mean that indeterminate exists under certain namespaces. /// - /// Meanwhile, if resolution is successful, the side effect of the resolution is returned. + /// Meanwhile, if resolution is successful, its result is returned. fn resolve_import<'r>( mut self: CmResolver<'r, 'ra, 'tcx>, import: Import<'ra>, - ) -> (Option>, usize) { + ) -> (Option>, usize) { debug!( "(resolving import for module) resolving import `{}::{}` in `{}`", Segment::names_to_string(&import.module_path), @@ -1070,18 +1047,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { (source, target, decls, type_ns_only) } ImportKind::Glob { .. } => { - return ( - Some(SideEffect { - imported_module: module, - decls: self.resolve_glob_import(import, module), - }), - 0, - ); + let import_resolution = ImportResolution { + imported_module: module, + kind: self.resolve_glob_import(import, module), + }; + return (Some(import_resolution), 0); } _ => unreachable!(), }; - let mut import_bindings = PerNS::default(); + let mut import_decls = PerNS::default(); let mut indeterminate_count = 0; self.per_ns_cm(|mut this, ns| { if !type_ns_only || ns == TypeNS { @@ -1099,24 +1074,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Ok(binding) => { // We need the `target`, `source` can be extracted. let import_decl = this.new_import_decl(binding, import); - Some(Some(import_decl)) + PendingDecl::Ready(Some(import_decl)) } - Err(Determinacy::Determined) => Some(None), + Err(Determinacy::Determined) => PendingDecl::Ready(None), Err(Determinacy::Undetermined) => { indeterminate_count += 1; - None + PendingDecl::Pending } }; - import_bindings[ns] = pending_binding; + import_decls[ns] = pending_binding; } }); - ( - Some(SideEffect { - imported_module: module, - decls: SideEffectDecls::Single { import_decls: import_bindings }, - }), - indeterminate_count, - ) + let import_resolution = ImportResolution { + imported_module: module, + kind: ImportResolutionKind::Single(import_decls), + }; + + (Some(import_resolution), indeterminate_count) } /// Performs final import resolution, consistency checks and error reporting. @@ -1671,7 +1645,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &self, import: Import<'ra>, imported_module: ModuleOrUniformRoot<'ra>, - ) -> SideEffectDecls<'ra> { + ) -> ImportResolutionKind<'ra> { match imported_module { ModuleOrUniformRoot::Module(module) if module != import.parent_scope.module => { let import_bindings = self @@ -1680,7 +1654,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { .iter() .filter_map(|(key, resolution)| { let res = resolution.borrow(); - let orig_ident_span = res.orig_ident_span; let binding = res.binding()?; let mut key = *key; let scope = match key.ident.ctxt.update_unchecked(|ctxt| { @@ -1690,16 +1663,19 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Some(None) => import.parent_scope.module, None => return None, }; - self.is_accessible_from(binding.vis.get(), scope) - .then(|| (binding, key, orig_ident_span)) + self.is_accessible_from(binding.vis.get(), scope).then_some(( + binding, + key, + res.orig_ident_span, + )) }) .collect::>(); - SideEffectDecls::Glob { imported_decls: import_bindings } + ImportResolutionKind::Glob(import_bindings) } - // Errors are reported in `commit_imports_resolutions` - _ => SideEffectDecls::None, + // Errors are reported in `write_imports_resolutions` + _ => ImportResolutionKind::Glob(vec![]), } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 6c354c24d99e3..15ece3b826920 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2661,7 +2661,7 @@ mod ref_mut { #[track_caller] pub(crate) fn get_mut(&mut self) -> &mut T { match self.mutable { - false => panic!("Can't mutably borrow speculative resolver"), + false => panic!("can't mutably borrow speculative resolver"), true => self.p, } } @@ -2688,12 +2688,12 @@ mod ref_mut { self.0.get() } - pub(crate) fn update_unchecked(&self, f: impl FnOnce(T) -> T) + pub(crate) fn update<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>, f: impl FnOnce(T) -> T) where T: Copy, { let old = self.get(); - self.set_unchecked(f(old)); + self.set(f(old), r); } } @@ -2704,15 +2704,11 @@ mod ref_mut { pub(crate) fn set<'ra, 'tcx>(&self, val: T, r: &Resolver<'ra, 'tcx>) { if r.assert_speculative { - panic!() + panic!("not allowed to mutate a `CmCell` during speculative resolution") } self.0.set(val); } - pub(crate) fn set_unchecked(&self, val: T) { - self.0.set(val); - } - pub(crate) fn into_inner(self) -> T { self.0.into_inner() } @@ -2727,7 +2723,6 @@ mod ref_mut { CmRefCell(RefCell::new(value)) } - #[track_caller] pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> { self.0.borrow_mut() } @@ -2735,16 +2730,29 @@ mod ref_mut { #[track_caller] pub(crate) fn borrow_mut<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> RefMut<'_, T> { if r.assert_speculative { - panic!("Not allowed to mutably borrow a CmRefCell during speculative resolution"); + panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); } - self.borrow_mut_unchecked() + self.0.borrow_mut() } - #[track_caller] pub(crate) fn try_borrow_mut_unchecked(&self) -> Result, BorrowMutError> { self.0.try_borrow_mut() } + #[track_caller] + pub(crate) fn try_borrow_mut<'ra, 'tcx>( + &self, + r: &Resolver<'ra, 'tcx>, + ) -> Result, BorrowMutError> { + let mut_borrow = self.0.try_borrow_mut(); + // We only check that this is allowed if we actually get the mutable reference. + if mut_borrow.is_ok() && r.assert_speculative { + panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); + } + + mut_borrow + } + #[track_caller] pub(crate) fn borrow(&self) -> Ref<'_, T> { self.0.borrow() @@ -2754,7 +2762,7 @@ mod ref_mut { impl CmRefCell { pub(crate) fn take<'ra, 'tcx>(&self, r: &Resolver<'ra, 'tcx>) -> T { if r.assert_speculative { - panic!("Not allowed to mutate a CmRefCell during speculative resolution"); + panic!("not allowed to mutate a CmRefCell during speculative resolution"); } self.0.take() } From b96410189482e63dcb7708413c3ed1442089c390 Mon Sep 17 00:00:00 2001 From: LorrensP-2158466 Date: Fri, 20 Feb 2026 13:27:00 +0100 Subject: [PATCH 3/3] address review --- compiler/rustc_resolve/src/imports.rs | 70 +++++++++++++-------------- compiler/rustc_resolve/src/lib.rs | 8 ++- 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index d5bca9cebddc4..786588edff6ab 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1036,15 +1036,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match path_res { PathResult::Module(module) => module, PathResult::Indeterminate => return (None, 3), - PathResult::NonModule(..) | PathResult::Failed { .. } => { - return (None, 0); - } + PathResult::NonModule(..) | PathResult::Failed { .. } => return (None, 0), } }; - let (source, _, bindings, type_ns_only) = match import.kind { - ImportKind::Single { source, target, ref decls, type_ns_only, .. } => { - (source, target, decls, type_ns_only) + let (source, bindings, type_ns_only) = match import.kind { + ImportKind::Single { source, ref decls, type_ns_only, .. } => { + (source, decls, type_ns_only) } ImportKind::Glob { .. } => { let import_resolution = ImportResolution { @@ -1070,7 +1068,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { &import.parent_scope, Some(import), ); - let pending_binding = match binding_result { + let pending_decl = match binding_result { Ok(binding) => { // We need the `target`, `source` can be extracted. let import_decl = this.new_import_decl(binding, import); @@ -1082,7 +1080,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { PendingDecl::Pending } }; - import_decls[ns] = pending_binding; + import_decls[ns] = pending_decl; } }); let import_resolution = ImportResolution { @@ -1646,37 +1644,35 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { import: Import<'ra>, imported_module: ModuleOrUniformRoot<'ra>, ) -> ImportResolutionKind<'ra> { - match imported_module { - ModuleOrUniformRoot::Module(module) if module != import.parent_scope.module => { - let import_bindings = self - .resolutions(module) - .borrow() - .iter() - .filter_map(|(key, resolution)| { - let res = resolution.borrow(); - let binding = res.binding()?; - let mut key = *key; - let scope = match key.ident.ctxt.update_unchecked(|ctxt| { - ctxt.reverse_glob_adjust(module.expansion, import.span) - }) { - Some(Some(def)) => self.expn_def_scope(def), - Some(None) => import.parent_scope.module, - None => return None, - }; - self.is_accessible_from(binding.vis.get(), scope).then_some(( - binding, - key, - res.orig_ident_span, - )) - }) - .collect::>(); - - ImportResolutionKind::Glob(import_bindings) - } + let import_bindings = match imported_module { + ModuleOrUniformRoot::Module(module) if module != import.parent_scope.module => self + .resolutions(module) + .borrow() + .iter() + .filter_map(|(key, resolution)| { + let res = resolution.borrow(); + let decl = res.binding()?; + let mut key = *key; + let scope = match key.ident.ctxt.update_unchecked(|ctxt| { + ctxt.reverse_glob_adjust(module.expansion, import.span) + }) { + Some(Some(def)) => self.expn_def_scope(def), + Some(None) => import.parent_scope.module, + None => return None, + }; + self.is_accessible_from(decl.vis.get(), scope).then_some(( + decl, + key, + res.orig_ident_span, + )) + }) + .collect::>(), // Errors are reported in `write_imports_resolutions` - _ => ImportResolutionKind::Glob(vec![]), - } + _ => vec![], + }; + + ImportResolutionKind::Glob(import_bindings) } // Miscellaneous post-processing, including recording re-exports, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 15ece3b826920..0f30b96ba5f80 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -2723,6 +2723,7 @@ mod ref_mut { CmRefCell(RefCell::new(value)) } + #[track_caller] pub(crate) fn borrow_mut_unchecked(&self) -> RefMut<'_, T> { self.0.borrow_mut() } @@ -2744,13 +2745,10 @@ mod ref_mut { &self, r: &Resolver<'ra, 'tcx>, ) -> Result, BorrowMutError> { - let mut_borrow = self.0.try_borrow_mut(); - // We only check that this is allowed if we actually get the mutable reference. - if mut_borrow.is_ok() && r.assert_speculative { + if r.assert_speculative { panic!("not allowed to mutably borrow a `CmRefCell` during speculative resolution"); } - - mut_borrow + self.0.try_borrow_mut() } #[track_caller]