Skip to content

Resolver: Batched Import Resolution #145108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Resolver: Batched Import Resolution #145108

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
77f5e5c
design of ImportResolver, which does the batched resolution
LorrensP-2158466 Aug 8, 2025
75c497c
isolate glob imports
LorrensP-2158466 Aug 8, 2025
9aa347e
isolate single import
LorrensP-2158466 Aug 8, 2025
71b10cd
bless tests
LorrensP-2158466 Aug 8, 2025
803cc87
nits
LorrensP-2158466 Aug 8, 2025
eda3931
prelude hack
LorrensP-2158466 Aug 10, 2025
3378f79
bless test
LorrensP-2158466 Aug 10, 2025
7e4b8d8
work around prelude hack
LorrensP-2158466 Aug 11, 2025
94bb516
address review
LorrensP-2158466 Aug 11, 2025
64d7519
refactor usage of ImportResolutionOutputs
LorrensP-2158466 Aug 11, 2025
4a2a0dc
resolve prelude import at `build_reduced_graph` phase
LorrensP-2158466 Aug 11, 2025
efb6489
update tests
LorrensP-2158466 Aug 12, 2025
fd413a3
address review
LorrensP-2158466 Aug 12, 2025
74c18d6
refactor ImportResolutionOutputs to use SideEffect
LorrensP-2158466 Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions compiler/rustc_hir/src/def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,24 @@ impl<T> PerNS<T> {
pub fn iter(&self) -> IntoIter<&T, 3> {
[&self.value_ns, &self.type_ns, &self.macro_ns].into_iter()
}

pub fn into_iter_with(self) -> IntoIter<(Namespace, T), 3> {
[
(Namespace::TypeNS, self.type_ns),
(Namespace::ValueNS, self.value_ns),
(Namespace::MacroNS, self.macro_ns),
]
.into_iter()
}

pub fn iter_with(&self) -> IntoIter<(Namespace, &T), 3> {
[
(Namespace::TypeNS, &self.type_ns),
(Namespace::ValueNS, &self.value_ns),
(Namespace::MacroNS, &self.macro_ns),
]
.into_iter()
}
}

impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1112,7 +1112,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
);
}
Scope::StdLibPrelude => {
if let Some(prelude) = this.prelude {
if let Some(prelude) = this.prelude.get() {
let mut tmp_suggestions = Vec::new();
this.add_module_candidates(prelude, &mut tmp_suggestions, filter_fn, None);
suggestions.extend(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/ident.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
},
Scope::StdLibPrelude => {
let mut result = Err(Determinacy::Determined);
if let Some(prelude) = this.prelude
if let Some(prelude) = this.prelude.get()
&& let Ok(binding) = this.reborrow().resolve_ident_in_module_unadjusted(
ModuleOrUniformRoot::Module(prelude),
ident,
Expand Down
177 changes: 130 additions & 47 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::cell::Cell;
use std::mem;
use std::ops::Deref;

use rustc_ast::NodeId;
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
Expand Down Expand Up @@ -41,6 +42,98 @@ use crate::{

type Res = def::Res<NodeId>;

pub(crate) struct ImportResolver<'r, 'ra, 'tcx> {
r: CmResolver<'r, 'ra, 'tcx>, // always immutable
batch: Vec<Import<'ra>>, // a.k.a. indeterminate_imports, also treated as output

// outputs
determined_imports: Vec<Import<'ra>>,
glob_import_outputs: Vec<(Module<'ra>, BindingKey, NameBinding<'ra>, bool)>,
glob_res_outputs: Vec<(NodeId, PartialRes)>,
import_bindings: PerNS<Vec<(Module<'ra>, Import<'ra>, PendingBinding<'ra>)>>,
}

struct ImportResolutionOutputs<'ra> {
indeterminate_imports: Vec<Import<'ra>>,
determined_imports: Vec<Import<'ra>>,
glob_import_outputs: Vec<(Module<'ra>, BindingKey, NameBinding<'ra>, bool)>,
glob_res_outputs: Vec<(NodeId, PartialRes)>,
import_bindings: PerNS<Vec<(Module<'ra>, Import<'ra>, PendingBinding<'ra>)>>,
}
Copy link
Contributor

@petrochenkov petrochenkov Aug 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest restructuring this structure like this:

struct ImportResolutionOutputs {
  indeterminate_imports: Vec<Import>,
  determined_imports: Vec<(Import, SideEffect)>,
}

where SideEffect is a set of changes that need to be applied for that specific import.

Then side effects will be applied in the same order in which the imports are resolved.
It will avoid having superfluous differences with the old algorithm, in emitted diagnostics in particular, and should make debugging the remaining differences easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, nice. That sounds better indeed, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way the diagnostics will also go in the source code order in common case, which is nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we would thus not need those changes to the stderr files of some tests, very nice.


impl<'r, 'ra, 'tcx> ImportResolver<'r, 'ra, 'tcx> {
pub(crate) fn new(cmr: CmResolver<'r, 'ra, 'tcx>, batch: Vec<Import<'ra>>) -> Self {
ImportResolver {
r: cmr,
batch,
determined_imports: Vec::new(),
import_bindings: PerNS::default(),
glob_import_outputs: Vec::new(),
glob_res_outputs: Vec::new(),
}
}

fn into_outputs(self) -> ImportResolutionOutputs<'ra> {
ImportResolutionOutputs {
indeterminate_imports: self.batch,
determined_imports: self.determined_imports,
import_bindings: self.import_bindings,
glob_import_outputs: self.glob_import_outputs,
glob_res_outputs: self.glob_res_outputs,
}
}
}

impl<'ra> ImportResolutionOutputs<'ra> {
fn commit<'tcx>(self, r: &mut Resolver<'ra, 'tcx>) {
r.indeterminate_imports = self.indeterminate_imports;
r.determined_imports.extend(self.determined_imports);

for (module, key, binding, warn_ambiguity) in self.glob_import_outputs {
let _ = r.try_define_local(module, key.ident.0, key.ns, binding, warn_ambiguity);
}

for (id, res) in self.glob_res_outputs {
r.record_partial_res(id, res);
}

for (ns, import_bindings) in self.import_bindings.into_iter_with() {
for (parent, import, pending_binding) in import_bindings {
let ImportKind::Single { target, ref bindings, .. } = import.kind else {
unreachable!();
};
match pending_binding {
PendingBinding::Ready(Some(binding)) => {
r.define_binding_local(parent, target, ns, binding);
}
PendingBinding::Ready(None) => {
let key = BindingKey::new(target, ns);
r.update_local_resolution(parent, key, false, |_, resolution| {
resolution.single_imports.swap_remove(&import);
});
}
_ => {}
}
bindings[ns].set(pending_binding);
}
}
}
}

impl<'r, 'ra, 'tcx> Deref for ImportResolver<'r, 'ra, 'tcx> {
type Target = Resolver<'ra, 'tcx>;

fn deref(&self) -> &Self::Target {
self.r.deref()
}
}

impl<'r, 'ra, 'tcx> AsRef<Resolver<'ra, 'tcx>> for ImportResolver<'r, 'ra, 'tcx> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is convenient to have the self: &mut ImportResolver methods because it makes the diff easier to read.
But right before merge we'll need to move the methods to ImportResolver itself, and remove the Deref/AsRef impls.

fn as_ref(&self) -> &Resolver<'ra, 'tcx> {
self.r.as_ref()
}
}

/// A [`NameBinding`] in the process of being resolved.
#[derive(Clone, Copy, Default, PartialEq)]
pub(crate) enum PendingBinding<'ra> {
Expand Down Expand Up @@ -552,22 +645,32 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
/// Resolves all imports for the crate. This method performs the fixed-
/// point iteration.
pub(crate) fn resolve_imports(&mut self) {
self.assert_speculative = true;
let mut prev_indeterminate_count = usize::MAX;
let mut indeterminate_count = self.indeterminate_imports.len() * 3;
while indeterminate_count < prev_indeterminate_count {
self.assert_speculative = true;
prev_indeterminate_count = indeterminate_count;
indeterminate_count = 0;
for import in mem::take(&mut self.indeterminate_imports) {
let 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),
}
let batch = mem::take(&mut self.indeterminate_imports);
let (outputs, count) = ImportResolver::new(self.cm(), batch).resolve_batch();
indeterminate_count = count;
self.assert_speculative = false;
outputs.commit(self);
}
}

fn resolve_batch<'r>(
mut self: ImportResolver<'r, 'ra, 'tcx>,
) -> (ImportResolutionOutputs<'ra>, usize) {
let mut indeterminate_count = 0;
for import in mem::take(&mut self.batch) {
let import_indeterminate_count = self.resolve_import(import);
indeterminate_count += import_indeterminate_count;
match import_indeterminate_count {
0 => self.determined_imports.push(import),
_ => self.batch.push(import),
}
}
self.assert_speculative = false;
(self.into_outputs(), indeterminate_count)
}

pub(crate) fn finalize_imports(&mut self) {
Expand Down Expand Up @@ -840,16 +943,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
///
/// 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 {
fn resolve_import<'r>(self: &mut ImportResolver<'r, 'ra, 'tcx>, import: Import<'ra>) -> 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() {
module
} else {
let path_res = self.reborrow().maybe_resolve_path(
let path_res = self.r.reborrow().maybe_resolve_path(
&import.module_path,
None,
&import.parent_scope,
Expand All @@ -869,16 +973,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
(source, target, bindings, type_ns_only)
}
ImportKind::Glob { .. } => {
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
self.get_mut_unchecked().resolve_glob_import(import);
self.resolve_glob_import(import);
return 0;
}
_ => unreachable!(),
};

let mut indeterminate_count = 0;
self.per_ns_cm(|this, ns| {
self.r.reborrow().per_ns_cm(|this, ns| {
if !type_ns_only || ns == TypeNS {
if bindings[ns].get() != PendingBinding::Pending {
return;
Expand All @@ -891,7 +993,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
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()
Expand All @@ -906,39 +1008,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
// We need the `target`, `source` can be extracted.
let imported_binding = this.import(binding, import);
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
this.get_mut_unchecked().define_binding_local(
parent,
target,
ns,
imported_binding,
);
PendingBinding::Ready(Some(imported_binding))
}
Err(Determinacy::Determined) => {
// Don't remove underscores from `single_imports`, they were never added.
if target.name != kw::Underscore {
let key = BindingKey::new(target, ns);
// FIXME: Use mutable resolver directly as a hack, this should be an output of
// specualtive resolution.
this.get_mut_unchecked().update_local_resolution(
parent,
key,
false,
|_, resolution| {
resolution.single_imports.swap_remove(&import);
},
);
if target.name == kw::Underscore {
return;
}
PendingBinding::Ready(None)
}
Err(Determinacy::Undetermined) => {
indeterminate_count += 1;
PendingBinding::Pending
return;
}
};
bindings[ns].set(binding);
self.import_bindings[ns].push((parent, import, pending_binding));
}
});

Expand Down Expand Up @@ -1483,7 +1567,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
false
}

fn resolve_glob_import(&mut self, import: Import<'ra>) {
fn resolve_glob_import<'r>(self: &mut ImportResolver<'r, 'ra, 'tcx>, import: Import<'ra>) {
// This function is only called for glob imports.
let ImportKind::Glob { id, is_prelude, .. } = import.kind else { unreachable!() };

Expand All @@ -1505,7 +1589,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if module == import.parent_scope.module {
return;
} else if is_prelude {
self.prelude = Some(module);
self.r.prelude.set(Some(module));
return;
}

Expand Down Expand Up @@ -1534,18 +1618,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
.resolution(import.parent_scope.module, key)
.and_then(|r| r.binding())
.is_some_and(|binding| binding.warn_ambiguity_recursive());
let _ = self.try_define_local(
self.glob_import_outputs.push((
import.parent_scope.module,
key.ident.0,
key.ns,
key,
imported_binding,
warn_ambiguity,
);
));
}
}

// Record the destination of this import
self.record_partial_res(id, PartialRes::new(module.res().unwrap()));
self.glob_res_outputs.push((id, PartialRes::new(module.res().unwrap())));
}

// Miscellaneous post-processing, including recording re-exports,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2485,7 +2485,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
.then_some(TypoSuggestion::typo_from_ident(ident.0, res))
}));

if let Some(prelude) = self.r.prelude {
if let Some(prelude) = self.r.prelude.get() {
self.r.add_module_candidates(prelude, &mut names, &filter_fn, None);
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,7 @@ pub struct Resolver<'ra, 'tcx> {
/// Assert that we are in speculative resolution mode.
assert_speculative: bool,

prelude: Option<Module<'ra>>,
prelude: Cell<Option<Module<'ra>>>,
extern_prelude: FxIndexMap<Macros20NormalizedIdent, ExternPreludeEntry<'ra>>,

/// N.B., this is used only for better diagnostics, not name resolution itself.
Expand Down Expand Up @@ -1533,7 +1533,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// AST.
graph_root,
assert_speculative: false, // Only set/cleared in Resolver::resolve_imports for now
prelude: None,
prelude: Cell::new(None),
extern_prelude,

field_names: Default::default(),
Expand Down Expand Up @@ -1881,7 +1881,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
this.get_mut().traits_in_module(module, assoc_item, &mut found_traits);
}
Scope::StdLibPrelude => {
if let Some(module) = this.prelude {
if let Some(module) = this.prelude.get() {
this.get_mut().traits_in_module(module, assoc_item, &mut found_traits);
}
}
Expand Down Expand Up @@ -2024,7 +2024,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if let ImportKind::MacroUse { warn_private: true } = import.kind {
// Do not report the lint if the macro name resolves in stdlib prelude
// even without the problematic `macro_use` import.
let found_in_stdlib_prelude = self.prelude.is_some_and(|prelude| {
let found_in_stdlib_prelude = self.prelude.get().is_some_and(|prelude| {
let empty_module = self.empty_module;
let arenas = self.arenas;
self.cm()
Expand Down Expand Up @@ -2504,7 +2504,7 @@ mod ref_mut {

/// 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 {
pub(crate) fn _get_mut_unchecked(&mut self) -> &mut T {
self.p
}
}
Expand Down
Loading
Loading