resolve: Preserve binding scopes in ambiguity errors

It allows to get rid of `AmbiguityErrorMisc` and `Flags`.
This commit is contained in:
Vadim Petrochenkov 2025-12-27 19:46:42 +03:00
parent 50db29bbd7
commit 1c7d3f0ff6
5 changed files with 85 additions and 126 deletions

View file

@ -4574,7 +4574,6 @@ dependencies = [
name = "rustc_resolve"
version = "0.0.0"
dependencies = [
"bitflags",
"indexmap",
"itertools",
"pulldown-cmark",

View file

@ -5,7 +5,6 @@ edition = "2024"
[dependencies]
# tidy-alphabetical-start
bitflags = "2.4.1"
indexmap = "2.4.0"
itertools = "0.12"
pulldown-cmark = { version = "0.11", features = ["html"], default-features = false }

View file

@ -44,7 +44,7 @@ use crate::errors::{
use crate::imports::{Import, ImportKind};
use crate::late::{DiagMetadata, PatternSource, Rib};
use crate::{
AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingError, BindingKey, Finalize,
AmbiguityError, AmbiguityKind, BindingError, BindingKey, Finalize,
ForwardGenericParamBanReason, HasGenericParams, LexicalScopeBinding, MacroRulesScope, Module,
ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult,
PrivacyError, ResolutionError, Resolver, Scope, ScopeSet, Segment, UseError, Used,
@ -1968,23 +1968,24 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
true
}
fn binding_description(&self, b: NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
fn binding_description(&self, b: NameBinding<'_>, ident: Ident, scope: Scope<'_>) -> String {
let res = b.res();
if b.span.is_dummy() || !self.tcx.sess.source_map().is_span_accessible(b.span) {
// These already contain the "built-in" prefix or look bad with it.
let add_built_in =
!matches!(b.res(), Res::NonMacroAttr(..) | Res::PrimTy(..) | Res::ToolMod);
let (built_in, from) = if from_prelude {
("", " from prelude")
} else if b.is_extern_crate()
&& !b.is_import()
&& self.tcx.sess.opts.externs.get(ident.as_str()).is_some()
{
("", " passed with `--extern`")
} else if add_built_in {
(" built-in", "")
} else {
("", "")
let (built_in, from) = match scope {
Scope::StdLibPrelude | Scope::MacroUsePrelude => ("", " from prelude"),
Scope::ExternPreludeFlags
if self.tcx.sess.opts.externs.get(ident.as_str()).is_some() =>
{
("", " passed with `--extern`")
}
_ => {
if matches!(res, Res::NonMacroAttr(..) | Res::PrimTy(..) | Res::ToolMod) {
// These already contain the "built-in" prefix or look bad with it.
("", "")
} else {
(" built-in", "")
}
}
};
let a = if built_in.is_empty() { res.article() } else { "a" };
@ -1996,22 +1997,24 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
fn ambiguity_diagnostic(&self, ambiguity_error: &AmbiguityError<'ra>) -> errors::Ambiguity {
let AmbiguityError { kind, ident, b1, b2, misc1, misc2, .. } = *ambiguity_error;
let AmbiguityError { kind, ident, b1, b2, scope1, scope2, .. } = *ambiguity_error;
let extern_prelude_ambiguity = || {
self.extern_prelude.get(&Macros20NormalizedIdent::new(ident)).is_some_and(|entry| {
entry.item_binding.map(|(b, _)| b) == Some(b1)
&& entry.flag_binding.as_ref().and_then(|pb| pb.get().0.binding()) == Some(b2)
})
// Note: b1 may come from a module scope, as an extern crate item in module.
matches!(scope2, Scope::ExternPreludeFlags)
&& self
.extern_prelude
.get(&Macros20NormalizedIdent::new(ident))
.is_some_and(|entry| entry.item_binding.map(|(b, _)| b) == Some(b1))
};
let (b1, b2, misc1, misc2, swapped) = if b2.span.is_dummy() && !b1.span.is_dummy() {
let (b1, b2, scope1, scope2, swapped) = if b2.span.is_dummy() && !b1.span.is_dummy() {
// We have to print the span-less alternative first, otherwise formatting looks bad.
(b2, b1, misc2, misc1, true)
(b2, b1, scope2, scope1, true)
} else {
(b1, b2, misc1, misc2, false)
(b1, b2, scope1, scope2, false)
};
let could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
let could_refer_to = |b: NameBinding<'_>, scope: Scope<'ra>, also: &str| {
let what = self.binding_description(b, ident, scope);
let note_msg = format!("`{ident}` could{also} refer to {what}");
let thing = b.res().descr();
@ -2029,12 +2032,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
{
help_msgs.push(format!("use `::{ident}` to refer to this {thing} unambiguously"))
}
match misc {
AmbiguityErrorMisc::SuggestCrate => help_msgs
.push(format!("use `crate::{ident}` to refer to this {thing} unambiguously")),
AmbiguityErrorMisc::SuggestSelf => help_msgs
.push(format!("use `self::{ident}` to refer to this {thing} unambiguously")),
AmbiguityErrorMisc::FromPrelude | AmbiguityErrorMisc::None => {}
if let Scope::Module(module, _) = scope {
if module == self.graph_root {
help_msgs.push(format!(
"use `crate::{ident}` to refer to this {thing} unambiguously"
));
} else if module != self.empty_module && module.is_normal() {
help_msgs.push(format!(
"use `self::{ident}` to refer to this {thing} unambiguously"
));
}
}
(
@ -2049,8 +2057,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
.collect::<Vec<_>>(),
)
};
let (b1_note, b1_help_msgs) = could_refer_to(b1, misc1, "");
let (b2_note, b2_help_msgs) = could_refer_to(b2, misc2, " also");
let (b1_note, b1_help_msgs) = could_refer_to(b1, scope1, "");
let (b2_note, b2_help_msgs) = could_refer_to(b2, scope2, " also");
errors::Ambiguity {
ident,

View file

@ -19,10 +19,10 @@ use crate::late::{
};
use crate::macros::{MacroRulesScope, sub_namespace_match};
use crate::{
AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingKey, CmResolver, Determinacy,
Finalize, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot,
NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res, ResolutionError,
Resolver, Scope, ScopeSet, Segment, Stage, Used, errors,
AmbiguityError, AmbiguityKind, BindingKey, CmResolver, Determinacy, Finalize, ImportKind,
LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind,
ParentScope, PathResult, PrivacyError, Res, ResolutionError, Resolver, Scope, ScopeSet,
Segment, Stage, Used, errors,
};
#[derive(Copy, Clone)]
@ -43,17 +43,6 @@ enum Shadowing {
Unrestricted,
}
bitflags::bitflags! {
#[derive(Clone, Copy)]
struct Flags: u8 {
const MACRO_RULES = 1 << 0;
const MODULE = 1 << 1;
const MISC_SUGGEST_CRATE = 1 << 2;
const MISC_SUGGEST_SELF = 1 << 3;
const MISC_FROM_PRELUDE = 1 << 4;
}
}
impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
/// A generic scope visitor.
/// Visits scopes in order to resolve some identifier in them or perform other actions.
@ -430,10 +419,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// }
// So we have to save the innermost solution and continue searching in outer scopes
// to detect potential ambiguities.
let mut innermost_result: Option<(NameBinding<'_>, Flags)> = None;
let mut innermost_result: Option<(NameBinding<'_>, Scope<'_>)> = None;
let mut determinacy = Determinacy::Determined;
let mut extern_prelude_item_binding = None;
let mut extern_prelude_flag_binding = None;
// Go through all the scopes and try to resolve the name.
let break_result = self.visit_scopes(
@ -459,11 +447,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ignore_binding,
ignore_import,
&mut extern_prelude_item_binding,
&mut extern_prelude_flag_binding,
)? {
Ok((binding, flags))
if sub_namespace_match(binding.macro_kinds(), macro_kind) =>
{
Ok(binding) if sub_namespace_match(binding.macro_kinds(), macro_kind) => {
// Below we report various ambiguity errors.
// We do not need to report them if we are either in speculative resolution,
// or in late resolution when everything is already imported and expanded
@ -472,24 +457,23 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
return ControlFlow::Break(Ok(binding));
}
if let Some((innermost_binding, innermost_flags)) = innermost_result {
if let Some((innermost_binding, innermost_scope)) = innermost_result {
// Found another solution, if the first one was "weak", report an error.
if this.get_mut().maybe_push_ambiguity(
orig_ident,
parent_scope,
binding,
innermost_binding,
flags,
innermost_flags,
scope,
innermost_scope,
extern_prelude_item_binding,
extern_prelude_flag_binding,
) {
// No need to search for more potential ambiguities, one is enough.
return ControlFlow::Break(Ok(innermost_binding));
}
} else {
// Found the first solution.
innermost_result = Some((binding, flags));
innermost_result = Some((binding, scope));
}
}
Ok(_) | Err(Determinacy::Determined) => {}
@ -507,7 +491,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Scope visiting walked all the scopes and maybe found something in one of them.
match innermost_result {
Some((binding, _)) => Ok(binding),
Some((binding, ..)) => Ok(binding),
None => Err(Determinacy::determined(determinacy == Determinacy::Determined || force)),
}
}
@ -526,18 +510,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ignore_binding: Option<NameBinding<'ra>>,
ignore_import: Option<Import<'ra>>,
extern_prelude_item_binding: &mut Option<NameBinding<'ra>>,
extern_prelude_flag_binding: &mut Option<NameBinding<'ra>>,
) -> ControlFlow<
Result<NameBinding<'ra>, Determinacy>,
Result<(NameBinding<'ra>, Flags), Determinacy>,
> {
) -> ControlFlow<Result<NameBinding<'ra>, Determinacy>, Result<NameBinding<'ra>, Determinacy>>
{
let ident = Ident::new(orig_ident.name, orig_ident.span.with_ctxt(ctxt));
let ret = match scope {
Scope::DeriveHelpers(expn_id) => {
if let Some(binding) = self.helper_attrs.get(&expn_id).and_then(|attrs| {
attrs.iter().rfind(|(i, _)| ident == *i).map(|(_, binding)| *binding)
}) {
Ok((binding, Flags::empty()))
Ok(binding)
} else {
Err(Determinacy::Determined)
}
@ -559,7 +540,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
derive.span,
LocalExpnId::ROOT,
);
result = Ok((binding, Flags::empty()));
result = Ok(binding);
break;
}
}
@ -573,7 +554,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
MacroRulesScope::Binding(macro_rules_binding)
if ident == macro_rules_binding.ident =>
{
Ok((macro_rules_binding.binding, Flags::MACRO_RULES))
Ok(macro_rules_binding.binding)
}
MacroRulesScope::Invocation(_) => Err(Determinacy::Undetermined),
_ => Err(Determinacy::Determined),
@ -612,14 +593,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
},
);
}
let misc_flags = if module == self.graph_root {
Flags::MISC_SUGGEST_CRATE
} else if module.is_normal() {
Flags::MISC_SUGGEST_SELF
} else {
Flags::empty()
};
Ok((binding, Flags::MODULE | misc_flags))
Ok(binding)
}
Err(ControlFlow::Continue(determinacy)) => Err(determinacy),
Err(ControlFlow::Break(Determinacy::Undetermined)) => {
@ -630,20 +604,20 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
}
Scope::MacroUsePrelude => match self.macro_use_prelude.get(&ident.name).cloned() {
Some(binding) => Ok((binding, Flags::MISC_FROM_PRELUDE)),
Some(binding) => Ok(binding),
None => Err(Determinacy::determined(
self.graph_root.unexpanded_invocations.borrow().is_empty(),
)),
},
Scope::BuiltinAttrs => match self.builtin_attrs_bindings.get(&ident.name) {
Some(binding) => Ok((*binding, Flags::empty())),
Some(binding) => Ok(*binding),
None => Err(Determinacy::Determined),
},
Scope::ExternPreludeItems => {
match self.reborrow().extern_prelude_get_item(ident, finalize.is_some()) {
Some(binding) => {
*extern_prelude_item_binding = Some(binding);
Ok((binding, Flags::empty()))
Ok(binding)
}
None => Err(Determinacy::determined(
self.graph_root.unexpanded_invocations.borrow().is_empty(),
@ -652,15 +626,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
Scope::ExternPreludeFlags => {
match self.extern_prelude_get_flag(ident, finalize.is_some()) {
Some(binding) => {
*extern_prelude_flag_binding = Some(binding);
Ok((binding, Flags::empty()))
}
Some(binding) => Ok(binding),
None => Err(Determinacy::Determined),
}
}
Scope::ToolPrelude => match self.registered_tool_bindings.get(&ident) {
Some(binding) => Ok((*binding, Flags::empty())),
Some(binding) => Ok(*binding),
None => Err(Determinacy::Determined),
},
Scope::StdLibPrelude => {
@ -679,7 +650,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
&& (matches!(use_prelude, UsePrelude::Yes)
|| self.is_builtin_macro(binding.res()))
{
result = Ok((binding, Flags::MISC_FROM_PRELUDE));
result = Ok(binding)
}
result
@ -712,7 +683,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
)
.emit();
}
Ok((*binding, Flags::empty()))
Ok(*binding)
}
None => Err(Determinacy::Determined),
},
@ -727,16 +698,17 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
parent_scope: &ParentScope<'ra>,
binding: NameBinding<'ra>,
innermost_binding: NameBinding<'ra>,
flags: Flags,
innermost_flags: Flags,
scope: Scope<'ra>,
innermost_scope: Scope<'ra>,
extern_prelude_item_binding: Option<NameBinding<'ra>>,
extern_prelude_flag_binding: Option<NameBinding<'ra>>,
) -> bool {
let (res, innermost_res) = (binding.res(), innermost_binding.res());
if res == innermost_res {
return false;
}
// FIXME: Use `scope` instead of `res` to detect built-in attrs and derive helpers,
// it will exclude imports, make slightly more code legal, and will require lang approval.
let is_builtin = |res| matches!(res, Res::NonMacroAttr(NonMacroAttrKind::Builtin(..)));
let derive_helper = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelper);
let derive_helper_compat = Res::NonMacroAttr(NonMacroAttrKind::DeriveHelperCompat);
@ -747,12 +719,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
Some(AmbiguityKind::DeriveHelper)
} else if res == derive_helper_compat && innermost_res != derive_helper {
span_bug!(orig_ident.span, "impossible inner resolution kind")
} else if innermost_flags.contains(Flags::MACRO_RULES)
&& flags.contains(Flags::MODULE)
} else if matches!(innermost_scope, Scope::MacroRules(_))
&& matches!(scope, Scope::Module(..))
&& !self.disambiguate_macro_rules_vs_modularized(innermost_binding, binding)
{
Some(AmbiguityKind::MacroRulesVsModularized)
} else if flags.contains(Flags::MACRO_RULES) && innermost_flags.contains(Flags::MODULE) {
} else if matches!(scope, Scope::MacroRules(_))
&& matches!(innermost_scope, Scope::Module(..))
{
// should be impossible because of visitation order in
// visit_scopes
//
@ -774,32 +748,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Skip ambiguity errors for extern flag bindings "overridden"
// by extern item bindings.
// FIXME: Remove with lang team approval.
let issue_145575_hack = Some(binding) == extern_prelude_flag_binding
let issue_145575_hack = matches!(scope, Scope::ExternPreludeFlags)
&& extern_prelude_item_binding.is_some()
&& extern_prelude_item_binding != Some(innermost_binding);
if issue_145575_hack {
self.issue_145575_hack_applied = true;
} else {
let misc = |f: Flags| {
if f.contains(Flags::MISC_SUGGEST_CRATE) {
AmbiguityErrorMisc::SuggestCrate
} else if f.contains(Flags::MISC_SUGGEST_SELF) {
AmbiguityErrorMisc::SuggestSelf
} else if f.contains(Flags::MISC_FROM_PRELUDE) {
AmbiguityErrorMisc::FromPrelude
} else {
AmbiguityErrorMisc::None
}
};
self.ambiguity_errors.push(AmbiguityError {
kind,
ident: orig_ident,
b1: innermost_binding,
b2: binding,
scope1: innermost_scope,
scope2: scope,
warning: false,
misc1: misc(innermost_flags),
misc2: misc(flags),
});
return true;
}
@ -1216,9 +1179,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ident,
b1: binding,
b2: shadowed_glob,
scope1: Scope::Module(self.empty_module, None),
scope2: Scope::Module(self.empty_module, None),
warning: false,
misc1: AmbiguityErrorMisc::None,
misc2: AmbiguityErrorMisc::None,
});
}

View file

@ -903,22 +903,14 @@ impl AmbiguityKind {
}
}
/// Miscellaneous bits of metadata for better ambiguity error reporting.
#[derive(Clone, Copy, PartialEq)]
enum AmbiguityErrorMisc {
SuggestCrate,
SuggestSelf,
FromPrelude,
None,
}
struct AmbiguityError<'ra> {
kind: AmbiguityKind,
ident: Ident,
b1: NameBinding<'ra>,
b2: NameBinding<'ra>,
misc1: AmbiguityErrorMisc,
misc2: AmbiguityErrorMisc,
// `empty_module` in module scope serves as an unknown module here.
scope1: Scope<'ra>,
scope2: Scope<'ra>,
warning: bool,
}
@ -2045,8 +2037,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
&& ambiguity_error.ident.span == ambi.ident.span
&& ambiguity_error.b1.span == ambi.b1.span
&& ambiguity_error.b2.span == ambi.b2.span
&& ambiguity_error.misc1 == ambi.misc1
&& ambiguity_error.misc2 == ambi.misc2
{
return true;
}
@ -2071,8 +2061,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
ident,
b1: used_binding,
b2,
misc1: AmbiguityErrorMisc::None,
misc2: AmbiguityErrorMisc::None,
scope1: Scope::Module(self.empty_module, None),
scope2: Scope::Module(self.empty_module, None),
warning: warn_ambiguity,
};
if !self.matches_previous_ambiguity_error(&ambiguity_error) {