bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

fix(resolve): update the ambiguity glob binding as warning recursively

Browse source
This commit is contained in:
bohan 2023-07-26 22:46:49 +08:00
parent 317ec04d18
commit cac0bd0bef
65 changed files with 1532 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

4
compiler/rustc_resolve/src/build_reduced_graph.rs
View file

@ -41,6 +41,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Module(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
@ -53,6 +54,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span,
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Res(self.0),
ambiguity: None,
warn_ambiguity: false,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
@ -69,7 +71,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
{
let binding = def.to_name_binding(self.arenas);
let key = self.new_disambiguated_key(ident, ns);
if let Err(old_binding) = self.try_define(parent, key, binding) {
if let Err(old_binding) = self.try_define(parent, key, binding, false) {
self.report_conflict(parent, ident, ns, old_binding, binding);
}
}

73
compiler/rustc_resolve/src/diagnostics.rs
View file

@ -5,10 +5,8 @@ use rustc_ast::{self as ast, Crate, ItemKind, ModKind, NodeId, Path, CRATE_NODE_
use rustc_ast::{MetaItemKind, NestedMetaItem};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{
pluralize, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan,
};
use rustc_errors::{struct_span_err, SuggestionStyle};
use rustc_errors::{pluralize, report_ambiguity_error, struct_span_err, SuggestionStyle};
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
use rustc_feature::BUILTIN_ATTRIBUTES;
use rustc_hir::def::Namespace::{self, *};
use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind, PerNS};
@ -17,8 +15,9 @@ use rustc_hir::PrimTy;
use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE;
use rustc_session::lint::builtin::AMBIGUOUS_GLOB_IMPORTS;
use rustc_session::lint::builtin::MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS;
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::lint::{AmbiguityErrorDiag, BuiltinLintDiagnostics};
use rustc_session::Session;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::edition::Edition;
@ -135,7 +134,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
for ambiguity_error in &self.ambiguity_errors {
self.report_ambiguity_error(ambiguity_error);
let diag = self.ambiguity_diagnostics(ambiguity_error);
if ambiguity_error.warning {
let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind else {
unreachable!()
};
self.lint_buffer.buffer_lint_with_diagnostic(
AMBIGUOUS_GLOB_IMPORTS,
import.root_id,
ambiguity_error.ident.span,
diag.msg.to_string(),
BuiltinLintDiagnostics::AmbiguousGlobImports { diag },
);
} else {
let mut err = struct_span_err!(self.tcx.sess, diag.span, E0659, "{}", &diag.msg);
report_ambiguity_error(&mut err, diag);
err.emit();
}
}
let mut reported_spans = FxHashSet::default();
@ -1540,20 +1555,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
fn report_ambiguity_error(&self, ambiguity_error: &AmbiguityError<'_>) {
let AmbiguityError { kind, ident, b1, b2, misc1, misc2 } = *ambiguity_error;
fn ambiguity_diagnostics(&self, ambiguity_error: &AmbiguityError<'_>) -> AmbiguityErrorDiag {
let AmbiguityError { kind, ident, b1, b2, misc1, misc2, .. } = *ambiguity_error;
let (b1, b2, misc1, misc2, 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)
} else {
(b1, b2, misc1, misc2, false)
};
let mut err = struct_span_err!(self.tcx.sess, ident.span, E0659, "`{ident}` is ambiguous");
err.span_label(ident.span, "ambiguous name");
err.note(format!("ambiguous because of {}", kind.descr()));
let mut could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
let note_msg = format!("`{ident}` could{also} refer to {what}");
@ -1579,16 +1589,35 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
AmbiguityErrorMisc::FromPrelude | AmbiguityErrorMisc::None => {}
}
err.span_note(b.span, note_msg);
for (i, help_msg) in help_msgs.iter().enumerate() {
let or = if i == 0 { "" } else { "or " };
err.help(format!("{}{}", or, help_msg));
}
(
b.span,
note_msg,
help_msgs
.iter()
.enumerate()
.map(|(i, help_msg)| {
let or = if i == 0 { "" } else { "or " };
format!("{}{}", or, help_msg)
})
.collect::<Vec<_>>(),
)
};
let (b1_span, b1_note_msg, b1_help_msgs) = could_refer_to(b1, misc1, "");
let (b2_span, b2_note_msg, b2_help_msgs) = could_refer_to(b2, misc2, " also");
could_refer_to(b1, misc1, "");
could_refer_to(b2, misc2, " also");
err.emit();
AmbiguityErrorDiag {
msg: format!("`{ident}` is ambiguous"),
span: ident.span,
label_span: ident.span,
label_msg: "ambiguous name".to_string(),
note_msg: format!("ambiguous because of {}", kind.descr()),
b1_span,
b1_note_msg,
b1_help_msgs,
b2_span,
b2_note_msg,
b2_help_msgs,
}
}
/// If the binding refers to a tuple struct constructor with fields,

2
compiler/rustc_resolve/src/ident.rs
View file

@ -677,6 +677,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident: orig_ident,
b1: innermost_binding,
b2: binding,
warning: false,
misc1: misc(innermost_flags),
misc2: misc(flags),
});
@ -905,6 +906,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident,
b1: binding,
b2: shadowed_glob,
warning: false,
misc1: AmbiguityErrorMisc::None,
misc2: AmbiguityErrorMisc::None,
});

114
compiler/rustc_resolve/src/imports.rs
View file

@ -284,6 +284,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Import { binding, import, used: Cell::new(false) },
ambiguity: None,
warn_ambiguity: false,
span: import.span,
vis,
expansion: import.parent_scope.expansion,
@ -291,16 +292,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
/// Define the name or return the existing binding if there is a collision.
/// `update` indicates if the definition is a redefinition of an existing binding.
pub(crate) fn try_define(
&mut self,
module: Module<'a>,
key: BindingKey,
binding: NameBinding<'a>,
warn_ambiguity: bool,
) -> Result<(), NameBinding<'a>> {
let res = binding.res();
self.check_reserved_macro_name(key.ident, res);
self.set_binding_parent_module(binding, module);
self.update_resolution(module, key, |this, resolution| {
self.update_resolution(module, key, warn_ambiguity, |this, resolution| {
if let Some(old_binding) = resolution.binding {
if res == Res::Err && old_binding.res() != Res::Err {
// Do not override real bindings with `Res::Err`s from error recovery.
@ -308,15 +311,42 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
match (old_binding.is_glob_import(), binding.is_glob_import()) {
(true, true) => {
if res != old_binding.res() {
resolution.binding = Some(this.ambiguity(
AmbiguityKind::GlobVsGlob,
old_binding,
binding,
));
// FIXME: remove `!binding.is_ambiguity()` after delete the warning ambiguity.
if !binding.is_ambiguity()
&& let NameBindingKind::Import { import: old_import, .. } = old_binding.kind
&& let NameBindingKind::Import { import, .. } = binding.kind
&& old_import == import {
// We should replace the `old_binding` with `binding` regardless
// of whether they has same resolution or not when they are
// imported from the same glob-import statement.
// However we currently using `Some(old_binding)` for back compact
// purposes.
// This case can be removed after once `Undetermined` is prepared
// for glob-imports.
} 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);
} 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()
}));
}
}
(old_glob @ true, false) | (old_glob @ false, true) => {
@ -374,29 +404,52 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
})
}
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()
})
}
// Use `f` to mutate the resolution of the name in the module.
// If the resolution becomes a success, define it in the module's glob importers.
fn update_resolution<T, F>(&mut self, module: Module<'a>, key: BindingKey, f: F) -> T
fn update_resolution<T, F>(
&mut self,
module: Module<'a>,
key: BindingKey,
warn_ambiguity: bool,
f: F,
) -> T
where
F: FnOnce(&mut Resolver<'a, 'tcx>, &mut NameResolution<'a>) -> T,
{
// 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) = {
let (binding, t, warn_ambiguity) = {
let resolution = &mut *self.resolution(module, key).borrow_mut();
let old_binding = resolution.binding();
let t = f(self, resolution);
if old_binding.is_none() && let Some(binding) = resolution.binding() {
(binding, t)
if let Some(binding) = resolution.binding() && old_binding != Some(binding) {
(binding, t, warn_ambiguity || old_binding.is_some())
} else {
return t;
}
};
// Define `binding` in `module`s glob importers.
for import in module.glob_importers.borrow_mut().iter() {
let Ok(glob_importers) = module.glob_importers.try_borrow_mut() else {
return t;
};
// Define or update `binding` in `module`s glob importers.
for import in glob_importers.iter() {
let mut ident = key.ident;
let scope = match ident.span.reverse_glob_adjust(module.expansion, import.span) {
Some(Some(def)) => self.expn_def_scope(def),
@ -406,7 +459,12 @@ impl<'a, 'tcx> Resolver<'a, '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(import.parent_scope.module, key, imported_binding);
let _ = self.try_define(
import.parent_scope.module,
key,
imported_binding,
warn_ambiguity,
);
}
}
@ -425,7 +483,7 @@ impl<'a, 'tcx> Resolver<'a, '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);
let _ = this.try_define(import.parent_scope.module, key, dummy_binding, false);
});
self.record_use(target, dummy_binding, false);
} else if import.imported_module.get().is_none() {
@ -700,7 +758,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Segment::names_to_string(&import.module_path),
module_to_string(import.parent_scope.module).unwrap_or_else(|| "???".to_string()),
);
let module = if let Some(module) = import.imported_module.get() {
module
} else {
@ -773,7 +830,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
.emit();
}
let key = BindingKey::new(target, ns);
this.update_resolution(parent, key, |_, resolution| {
this.update_resolution(parent, key, false, |_, resolution| {
resolution.single_imports.remove(&import);
});
}
@ -989,7 +1046,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
initial_binding.res()
});
let res = binding.res();
if res == Res::Err || !this.ambiguity_errors.is_empty() {
let has_ambiguity_error = this
.ambiguity_errors
.iter()
.filter(|error| !error.warning)
.next()
.is_some();
if res == Res::Err || has_ambiguity_error {
this.tcx
.sess
.delay_span_bug(import.span, "some error happened for an import");
@ -1338,7 +1401,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
};
if self.is_accessible_from(binding.vis, scope) {
let imported_binding = self.import(binding, import);
let _ = self.try_define(import.parent_scope.module, key, imported_binding);
let warn_ambiguity = self
.resolution(import.parent_scope.module, key)
.borrow()
.binding()
.is_some_and(|binding| binding.is_warn_ambiguity());
let _ = self.try_define(
import.parent_scope.module,
key,
imported_binding,
warn_ambiguity,
);
}
}
@ -1357,7 +1430,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
module.for_each_child(self, |this, ident, _, binding| {
let res = binding.res().expect_non_local();
if res != def::Res::Err && !binding.is_ambiguity() {
let error_ambiguity = binding.is_ambiguity() && !binding.warn_ambiguity;
if res != def::Res::Err && !error_ambiguity {
let mut reexport_chain = SmallVec::new();
let mut next_binding = binding;
while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {

26
compiler/rustc_resolve/src/lib.rs
View file

@ -658,6 +658,7 @@ impl<'a> fmt::Debug for Module<'a> {
struct NameBindingData<'a> {
kind: NameBindingKind<'a>,
ambiguity: Option<(NameBinding<'a>, AmbiguityKind)>,
warn_ambiguity: bool,
expansion: LocalExpnId,
span: Span,
vis: ty::Visibility<DefId>,
@ -767,6 +768,7 @@ struct AmbiguityError<'a> {
b2: NameBinding<'a>,
misc1: AmbiguityErrorMisc,
misc2: AmbiguityErrorMisc,
warning: bool,
}
impl<'a> NameBindingData<'a> {
@ -794,6 +796,14 @@ impl<'a> NameBindingData<'a> {
}
}
fn is_warn_ambiguity(&self) -> bool {
self.warn_ambiguity
|| match self.kind {
NameBindingKind::Import { binding, .. } => binding.is_warn_ambiguity(),
_ => false,
}
}
fn is_possibly_imported_variant(&self) -> bool {
match self.kind {
NameBindingKind::Import { binding, .. } => binding.is_possibly_imported_variant(),
@ -1322,6 +1332,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
dummy_binding: arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Res(Res::Err),
ambiguity: None,
warn_ambiguity: false,
expansion: LocalExpnId::ROOT,
span: DUMMY_SP,
vis: ty::Visibility::Public,
@ -1685,6 +1696,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
fn record_use(&mut self, ident: Ident, used_binding: NameBinding<'a>, is_lexical_scope: bool) {
self.record_use_inner(ident, used_binding, is_lexical_scope, used_binding.warn_ambiguity);
}
fn record_use_inner(
&mut self,
ident: Ident,
used_binding: NameBinding<'a>,
is_lexical_scope: bool,
warn_ambiguity: bool,
) {
if let Some((b2, kind)) = used_binding.ambiguity {
let ambiguity_error = AmbiguityError {
kind,
@ -1693,9 +1714,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
b2,
misc1: AmbiguityErrorMisc::None,
misc2: AmbiguityErrorMisc::None,
warning: warn_ambiguity,
};
if !self.matches_previous_ambiguity_error(&ambiguity_error) {
// avoid duplicated span information to be emitt out
// avoid duplicated span information to be emit out
self.ambiguity_errors.push(ambiguity_error);
}
}
@ -1715,7 +1737,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.used_imports.insert(id);
}
self.add_to_glob_map(import, ident);
self.record_use(ident, binding, false);
self.record_use_inner(ident, binding, false, warn_ambiguity || binding.warn_ambiguity);
}
}