Skip to content

[WIP] Resolve: refactor define into define_local and define_extern #143884

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.

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

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

LorrensP-2158466
Copy link
Contributor

@LorrensP-2158466 LorrensP-2158466 commented Jul 13, 2025

Follow up on #143550 and part of #gsoc > Project: Parallel Macro Expansion.

Split up define into define_local and define_extern. Refactor usages of define into either one where it's "correct" (idk if everything is correct atm). Big part of this is that resolution can now take a &Resolver instead of a mutable one.

Needed the module changes of #143550. Review will probably be easier when viewing my first commit instead of all the changes.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 13, 2025
panic!("An external binding was already defined");
}
// FIXME: maybe some handling of glob-importers
resolution.binding = Some(binding);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything passes but I still don't know what we should do here with glob-importers.

@@ -477,7 +492,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if self.is_accessible_from(binding.vis, scope) {
let imported_binding = self.import(binding, *import);
let key = BindingKey { ident, ..key };
let _ = self.try_define(
// FIXME: local or external?
let _ = self.try_define_local(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in update_resolutions, I don't know if this is an equivalent change.

@@ -1078,7 +1078,7 @@ pub struct Resolver<'ra, 'tcx> {
extern_module_map: RefCell<FxIndexMap<DefId, Module<'ra>>>,
binding_parent_modules: FxHashMap<NameBinding<'ra>, Module<'ra>>,

underscore_disambiguator: u32,
underscore_disambiguator: Cell<u32>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is correct, right?

@@ -501,7 +517,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let dummy_binding = self.import(dummy_binding, import);
self.per_ns(|this, ns| {
let key = BindingKey::new(target, ns);
let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false);
let _ =
this.try_define_local(import.parent_scope.module, key, dummy_binding, false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same concern as in update_resolutions.

@bors
Copy link
Collaborator

bors commented Jul 13, 2025

☔ The latest upstream changes (presumably #143888) made this pull request unmergeable. Please resolve the merge conflicts.

@LorrensP-2158466
Copy link
Contributor Author

Conflicts must be resolved in Petrochenkov's PRS, I think. I can then rebase off that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants