Skip to content
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

resolve: Tweak some naming around import ambiguities #126954

Merged
merged 1 commit into from
Jun 26, 2024
Merged

resolve: Tweak some naming around import ambiguities #126954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/effective_visibilities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
// is the maximum value among visibilities of bindings corresponding to that def id.
for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {
let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
if !binding.is_ambiguity() {
if !binding.is_ambiguity_recursive() {
if let Some(node_id) = import.id() {
r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
}
Expand Down
74 changes: 32 additions & 42 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
match (old_binding.is_glob_import(), binding.is_glob_import()) {
(true, true) => {
// FIXME: remove `!binding.is_ambiguity()` after delete the warning ambiguity.
if !binding.is_ambiguity()
// FIXME: remove `!binding.is_ambiguity_recursive()` after delete the warning ambiguity.
if !binding.is_ambiguity_recursive()
&& let NameBindingKind::Import { import: old_import, .. } =
old_binding.kind
&& let NameBindingKind::Import { import, .. } = binding.kind
Expand All @@ -337,21 +337,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// imported from the same glob-import statement.
resolution.binding = Some(binding);
} else if res != old_binding.res() {
let binding = if warn_ambiguity {
this.warn_ambiguity(AmbiguityKind::GlobVsGlob, old_binding, binding)
} else {
this.ambiguity(AmbiguityKind::GlobVsGlob, old_binding, binding)
};
resolution.binding = Some(binding);
resolution.binding = Some(this.new_ambiguity_binding(
AmbiguityKind::GlobVsGlob,
old_binding,
binding,
warn_ambiguity,
));
} else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
// We are glob-importing the same item but with greater visibility.
resolution.binding = Some(binding);
} else if binding.is_ambiguity() {
resolution.binding =
Some(self.arenas.alloc_name_binding(NameBindingData {
warn_ambiguity: true,
..(*binding).clone()
}));
} else if binding.is_ambiguity_recursive() {
resolution.binding = Some(this.new_warn_ambiguity_binding(binding));
}
}
(old_glob @ true, false) | (old_glob @ false, true) => {
Expand All @@ -361,24 +357,26 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&& nonglob_binding.expansion != LocalExpnId::ROOT
&& glob_binding.res() != nonglob_binding.res()
{
resolution.binding = Some(this.ambiguity(
resolution.binding = Some(this.new_ambiguity_binding(
AmbiguityKind::GlobVsExpanded,
nonglob_binding,
glob_binding,
false,
));
} else {
resolution.binding = Some(nonglob_binding);
}

if let Some(old_binding) = resolution.shadowed_glob {
assert!(old_binding.is_glob_import());
if glob_binding.res() != old_binding.res() {
resolution.shadowed_glob = Some(this.ambiguity(
if let Some(old_shadowed_glob) = resolution.shadowed_glob {
assert!(old_shadowed_glob.is_glob_import());
if glob_binding.res() != old_shadowed_glob.res() {
resolution.shadowed_glob = Some(this.new_ambiguity_binding(
AmbiguityKind::GlobVsGlob,
old_binding,
old_shadowed_glob,
glob_binding,
false,
));
} else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
} else if !old_shadowed_glob.vis.is_at_least(binding.vis, this.tcx) {
resolution.shadowed_glob = Some(glob_binding);
}
} else {
Expand All @@ -397,29 +395,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
})
}

fn ambiguity(
fn new_ambiguity_binding(
&self,
kind: AmbiguityKind,
ambiguity_kind: AmbiguityKind,
primary_binding: NameBinding<'a>,
secondary_binding: NameBinding<'a>,
warn_ambiguity: bool,
) -> NameBinding<'a> {
self.arenas.alloc_name_binding(NameBindingData {
ambiguity: Some((secondary_binding, kind)),
..(*primary_binding).clone()
})
let ambiguity = Some((secondary_binding, ambiguity_kind));
let data = NameBindingData { ambiguity, warn_ambiguity, ..*primary_binding };
self.arenas.alloc_name_binding(data)
}

fn warn_ambiguity(
&self,
kind: AmbiguityKind,
primary_binding: NameBinding<'a>,
secondary_binding: NameBinding<'a>,
) -> NameBinding<'a> {
self.arenas.alloc_name_binding(NameBindingData {
ambiguity: Some((secondary_binding, kind)),
warn_ambiguity: true,
..(*primary_binding).clone()
})
fn new_warn_ambiguity_binding(&self, binding: NameBinding<'a>) -> NameBinding<'a> {
assert!(binding.is_ambiguity_recursive());
self.arenas.alloc_name_binding(NameBindingData { warn_ambiguity: true, ..*binding })
}

// Use `f` to mutate the resolution of the name in the module.
Expand Down Expand Up @@ -1381,8 +1371,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
target_bindings[ns].get(),
) {
Ok(other_binding) => {
is_redundant =
binding.res() == other_binding.res() && !other_binding.is_ambiguity();
is_redundant = binding.res() == other_binding.res()
&& !other_binding.is_ambiguity_recursive();
if is_redundant {
redundant_span[ns] =
Some((other_binding.span, other_binding.is_import()));
Expand Down Expand Up @@ -1455,7 +1445,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
.resolution(import.parent_scope.module, key)
.borrow()
.binding()
.is_some_and(|binding| binding.is_warn_ambiguity());
.is_some_and(|binding| binding.warn_ambiguity_recursive());
let _ = self.try_define(
import.parent_scope.module,
key,
Expand All @@ -1480,7 +1470,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

module.for_each_child(self, |this, ident, _, binding| {
let res = binding.res().expect_non_local();
let error_ambiguity = binding.is_ambiguity() && !binding.warn_ambiguity;
let error_ambiguity = binding.is_ambiguity_recursive() && !binding.warn_ambiguity;
if res != def::Res::Err && !error_ambiguity {
let mut reexport_chain = SmallVec::new();
let mut next_binding = binding;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/late.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3730,7 +3730,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
let ls_binding = self.maybe_resolve_ident_in_lexical_scope(ident, ValueNS)?;
let (res, binding) = match ls_binding {
LexicalScopeBinding::Item(binding)
if is_syntactic_ambiguity && binding.is_ambiguity() =>
if is_syntactic_ambiguity && binding.is_ambiguity_recursive() =>
{
// For ambiguous bindings we don't know all their definitions and cannot check
// whether they can be shadowed by fresh bindings or not, so force an error.
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,10 +694,12 @@ impl<'a> fmt::Debug for Module<'a> {
}

/// Records a possibly-private value, type, or module definition.
#[derive(Clone, Debug)]
#[derive(Clone, Copy, Debug)]
struct NameBindingData<'a> {
kind: NameBindingKind<'a>,
ambiguity: Option<(NameBinding<'a>, AmbiguityKind)>,
/// Produce a warning instead of an error when reporting ambiguities inside this binding.
/// May apply to indirect ambiguities under imports, so `ambiguity.is_some()` is not required.
warn_ambiguity: bool,
expansion: LocalExpnId,
span: Span,
Expand All @@ -718,7 +720,7 @@ impl<'a> ToNameBinding<'a> for NameBinding<'a> {
}
}

#[derive(Clone, Debug)]
#[derive(Clone, Copy, Debug)]
enum NameBindingKind<'a> {
Res(Res),
Module(Module<'a>),
Expand Down Expand Up @@ -830,18 +832,18 @@ impl<'a> NameBindingData<'a> {
}
}

fn is_ambiguity(&self) -> bool {
fn is_ambiguity_recursive(&self) -> bool {
self.ambiguity.is_some()
|| match self.kind {
NameBindingKind::Import { binding, .. } => binding.is_ambiguity(),
NameBindingKind::Import { binding, .. } => binding.is_ambiguity_recursive(),
_ => false,
}
}

fn is_warn_ambiguity(&self) -> bool {
fn warn_ambiguity_recursive(&self) -> bool {
self.warn_ambiguity
|| match self.kind {
NameBindingKind::Import { binding, .. } => binding.is_warn_ambiguity(),
NameBindingKind::Import { binding, .. } => binding.warn_ambiguity_recursive(),
_ => false,
}
}
Expand Down
Loading