Skip to content

Commit

Permalink
Point out shadowed associated types
Browse files Browse the repository at this point in the history
When trying to set the value of a shadowed associated type,
the error message was confusing, as it simply said that there is a
missing type. Now the compiler notifies the user about the shadow,
and suggests renaming as a fix
  • Loading branch information
TheLazyDutchman committed Nov 7, 2023
1 parent 9c8a269 commit 6d8c642
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 1 deletion.
61 changes: 60 additions & 1 deletion compiler/rustc_hir_analysis/src/astconv/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorG
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::traits::FulfillmentError;
use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt};
use rustc_middle::ty::{self, suggest_constraining_type_param, AssocItem, AssocKind, Ty, TyCtxt};
use rustc_session::parse::feature_err;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::symbol::{sym, Ident};
Expand Down Expand Up @@ -513,6 +513,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
if associated_types.values().all(|v| v.is_empty()) {
return;
}

let tcx = self.tcx();
// FIXME: Marked `mut` so that we can replace the spans further below with a more
// appropriate one, but this should be handled earlier in the span assignment.
Expand Down Expand Up @@ -585,6 +586,35 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
}

// We get all the associated items that _are_ set,
// so that we can check if any of their names match one of the ones we are missing.
// This would mean that they are shadowing the associated type we are missing,
// and we can then use their span to indicate this to the user.
let bound_names = trait_bounds
.iter()
.filter_map(|poly_trait_ref| {
poly_trait_ref.trait_ref.path.segments.last().and_then(|path| {
let args = path.args?;
Some((path, args))
})
})
.flat_map(|(path, args)| {
args.bindings.iter().map(|binding| {
let ident = binding.ident;
let trait_def = path.res.opt_def_id();
let assoc_item = trait_def.and_then(|did| {
tcx.associated_items(did).find_by_name_and_kinds(
tcx,
ident,
&[AssocKind::Fn, AssocKind::Type, AssocKind::Const],
did,
)
});
(ident.name, assoc_item)
})
})
.collect::<FxHashMap<Symbol, Option<&AssocItem>>>();

let mut names = names
.into_iter()
.map(|(trait_, mut assocs)| {
Expand Down Expand Up @@ -614,6 +644,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
pluralize!(names_len),
names,
);
let mut rename_suggestions = vec![];
let mut suggestions = vec![];
let mut types_count = 0;
let mut where_constraints = vec![];
Expand All @@ -625,23 +656,47 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
*names.entry(item.name).or_insert(0) += 1;
}
let mut dupes = false;
let mut shadows = false;
for item in assoc_items {
let prefix = if names[&item.name] > 1 {
let trait_def_id = item.container_id(tcx);
dupes = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else if bound_names.contains_key(&item.name) {
let trait_def_id = item.container_id(tcx);
shadows = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else {
String::new()
};
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
err.span_label(sp, format!("`{}{}` defined here", prefix, item.name));
}

if let Some(Some(assoc_item)) = bound_names.get(&item.name) {
err.span_label(
tcx.def_span(assoc_item.def_id),
format!("`{}{}` shadowed here", prefix, item.name),
);
}
}
if potential_assoc_types.len() == assoc_items.len() {
// When the amount of missing associated types equals the number of
// extra type arguments present. A suggesting to replace the generic args with
// associated types is already emitted.
already_has_generics_args_suggestion = true;
} else if shadows {
for item in assoc_items {
if let Some(Some(assoc_item)) = bound_names.get(&item.name) {
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
rename_suggestions.push(sp);
}

if let Some(sp) = tcx.hir().span_if_local(assoc_item.def_id) {
rename_suggestions.push(sp);
}
}
}
} else if let (Ok(snippet), false) =
(tcx.sess.source_map().span_to_snippet(*span), dupes)
{
Expand Down Expand Up @@ -725,6 +780,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
err.span_help(where_constraints, where_msg);
}
}

for span in rename_suggestions {
err.span_help(span, "consider renaming this associated type");
}
err.emit();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | inner: <IntoIterator<Item: IntoIterator<Item: >>::IntoIterator as Item>
| | |
| | associated types `Item`, `IntoIter` must be specified
| associated types `Item`, `IntoIter` must be specified
--> $SRC_DIR/core/src/iter/traits/collect.rs:LL:COL
|
= note: `IntoIterator::Item` shadowed here
|
= note: `IntoIterator::Item` shadowed here

error[E0223]: ambiguous associated type
--> $DIR/overlaping-bound-suggestion.rs:7:13
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Test Setting the value of an associated type
// that is shadowed from a supertrait

pub trait Super {
type X;
}

pub trait Sub: Super {
type X;
}

impl<T> Clone for Box<dyn Sub<X = T>> {
//~^ ERROR associated type `X` in `Super` must be specified
fn clone(&self) -> Self {
unimplemented!();
}
}

pub fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0191]: the value of the associated type `X` in `Super` must be specified
--> $DIR/associated-type-shadowed-from-supertrait.rs:12:27
|
LL | type X;
| ------ `Super::X` defined here
...
LL | type X;
| ------ `Super::X` shadowed here
...
LL | impl<T> Clone for Box<dyn Sub<X = T>> {
| ^^^^^^^^^^ associated type `X` must be specified
|
help: consider renaming this associated type
--> $DIR/associated-type-shadowed-from-supertrait.rs:5:5
|
LL | type X;
| ^^^^^^
help: consider renaming this associated type
--> $DIR/associated-type-shadowed-from-supertrait.rs:9:5
|
LL | type X;
| ^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0191`.

0 comments on commit 6d8c642

Please sign in to comment.