bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Dont use span as key when collecting missing associated types from dyn

Browse source
This commit is contained in:
Michael Goulet 2024-11-23 18:10:47 +00:00
parent 1f3bf231e1
commit 25a7fdf5bc
2 changed files with 134 additions and 149 deletions
Download patch file Download diff file Expand all files Collapse all files

41
compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs
View file

@ -1,9 +1,8 @@
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_errors::codes::*; use rustc_errors::codes::*;
use rustc_errors::struct_span_code_err; use rustc_errors::struct_span_code_err;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res}; use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS; use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
use rustc_middle::span_bug; use rustc_middle::span_bug;
use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::fold::BottomUpFolder;
@ -11,7 +10,7 @@ use rustc_middle::ty::{
self, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable, self, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
TypeVisitableExt, Upcast, TypeVisitableExt, Upcast,
}; };
use rustc_span::{ErrorGuaranteed, Span}; use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility; use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility;
use rustc_trait_selection::traits::{self, hir_ty_lowering_dyn_compatibility_violations}; use rustc_trait_selection::traits::{self, hir_ty_lowering_dyn_compatibility_violations};
use rustc_type_ir::elaborate::ClauseWithSupertraitSpan; use rustc_type_ir::elaborate::ClauseWithSupertraitSpan;
@ -128,8 +127,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
} }
} }
let mut associated_types: FxIndexMap<Span, FxIndexSet<DefId>> = FxIndexMap::default(); let mut needed_associated_types = FxIndexSet::default();
let principal_span = regular_traits.first().map_or(DUMMY_SP, |info| info.bottom().1);
let regular_traits_refs_spans = trait_bounds let regular_traits_refs_spans = trait_bounds
.into_iter() .into_iter()
.filter(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id())); .filter(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id()));
@ -146,11 +146,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
match bound_predicate.skip_binder() { match bound_predicate.skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => { ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => {
let pred = bound_predicate.rebind(pred); let pred = bound_predicate.rebind(pred);
associated_types.entry(original_span).or_default().extend( needed_associated_types.extend(
tcx.associated_items(pred.def_id()) tcx.associated_items(pred.def_id())
.in_definition_order() .in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type) .filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| !item.is_impl_trait_in_trait()) .filter(|item| !item.is_impl_trait_in_trait())
// If the associated type has a `where Self: Sized` bound,
// we do not need to constrain the associated type.
.filter(|item| !tcx.generics_require_sized_self(item.def_id))
.map(|item| item.def_id), .map(|item| item.def_id),
); );
} }
@ -201,26 +204,22 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated
// types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
// corresponding `Projection` clause // corresponding `Projection` clause
for def_ids in associated_types.values_mut() { for (projection_bound, span) in &projection_bounds {
for (projection_bound, span) in &projection_bounds { let def_id = projection_bound.item_def_id();
let def_id = projection_bound.item_def_id(); needed_associated_types.swap_remove(&def_id);
def_ids.swap_remove(&def_id); if tcx.generics_require_sized_self(def_id) {
if tcx.generics_require_sized_self(def_id) { tcx.emit_node_span_lint(
tcx.emit_node_span_lint( UNUSED_ASSOCIATED_TYPE_BOUNDS,
UNUSED_ASSOCIATED_TYPE_BOUNDS, hir_id,
hir_id, *span,
*span, crate::errors::UnusedAssociatedTypeBounds { span: *span },
crate::errors::UnusedAssociatedTypeBounds { span: *span }, );
);
}
} }
// If the associated type has a `where Self: Sized` bound, we do not need to constrain the associated
// type in the `dyn Trait`.
def_ids.retain(|def_id| !tcx.generics_require_sized_self(def_id));
} }
if let Err(guar) = self.check_for_required_assoc_tys( if let Err(guar) = self.check_for_required_assoc_tys(
associated_types, principal_span,
needed_associated_types,
potential_assoc_types, potential_assoc_types,
hir_trait_bounds, hir_trait_bounds,
) { ) {

242
compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file

@ -722,51 +722,40 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// emit a generic note suggesting using a `where` clause to constraint instead. /// emit a generic note suggesting using a `where` clause to constraint instead.
pub(crate) fn check_for_required_assoc_tys( pub(crate) fn check_for_required_assoc_tys(
&self, &self,
associated_types: FxIndexMap<Span, FxIndexSet<DefId>>, principal_span: Span,
missing_assoc_types: FxIndexSet<DefId>,
potential_assoc_types: Vec<usize>, potential_assoc_types: Vec<usize>,
trait_bounds: &[hir::PolyTraitRef<'_>], trait_bounds: &[hir::PolyTraitRef<'_>],
) -> Result<(), ErrorGuaranteed> { ) -> Result<(), ErrorGuaranteed> {
if associated_types.values().all(|v| v.is_empty()) { if missing_assoc_types.is_empty() {
return Ok(()); return Ok(());
} }
let tcx = self.tcx(); let tcx = self.tcx();
// FIXME: Marked `mut` so that we can replace the spans further below with a more let missing_assoc_types: Vec<_> =
// appropriate one, but this should be handled earlier in the span assignment. missing_assoc_types.into_iter().map(|def_id| tcx.associated_item(def_id)).collect();
let associated_types: FxIndexMap<Span, Vec<_>> = associated_types
.into_iter()
.map(|(span, def_ids)| {
(span, def_ids.into_iter().map(|did| tcx.associated_item(did)).collect())
})
.collect();
let mut names: FxIndexMap<String, Vec<Symbol>> = Default::default(); let mut names: FxIndexMap<String, Vec<Symbol>> = Default::default();
let mut names_len = 0; let mut names_len = 0;
// Account for things like `dyn Foo + 'a`, like in tests `issue-22434.rs` and // Account for things like `dyn Foo + 'a`, like in tests `issue-22434.rs` and
// `issue-22560.rs`. // `issue-22560.rs`.
let mut trait_bound_spans: Vec<Span> = vec![];
let mut dyn_compatibility_violations = Ok(()); let mut dyn_compatibility_violations = Ok(());
for (span, items) in &associated_types { for assoc_item in &missing_assoc_types {
if !items.is_empty() { let trait_def_id = assoc_item.container_id(tcx);
trait_bound_spans.push(*span); names.entry(tcx.def_path_str(trait_def_id)).or_default().push(assoc_item.name);
} names_len += 1;
for assoc_item in items {
let trait_def_id = assoc_item.container_id(tcx);
names.entry(tcx.def_path_str(trait_def_id)).or_default().push(assoc_item.name);
names_len += 1;
let violations = let violations =
dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item); dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item);
if !violations.is_empty() { if !violations.is_empty() {
dyn_compatibility_violations = Err(report_dyn_incompatibility( dyn_compatibility_violations = Err(report_dyn_incompatibility(
tcx, tcx,
*span, principal_span,
None, None,
trait_def_id, trait_def_id,
&violations, &violations,
) )
.emit()); .emit());
}
} }
} }
@ -827,10 +816,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
names.sort(); names.sort();
let names = names.join(", "); let names = names.join(", ");
trait_bound_spans.sort();
let mut err = struct_span_code_err!( let mut err = struct_span_code_err!(
self.dcx(), self.dcx(),
trait_bound_spans, principal_span,
E0191, E0191,
"the value of the associated type{} {} must be specified", "the value of the associated type{} {} must be specified",
pluralize!(names_len), pluralize!(names_len),
@ -840,81 +828,81 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let mut types_count = 0; let mut types_count = 0;
let mut where_constraints = vec![]; let mut where_constraints = vec![];
let mut already_has_generics_args_suggestion = false; let mut already_has_generics_args_suggestion = false;
for (span, assoc_items) in &associated_types {
let mut names: UnordMap<_, usize> = Default::default();
for item in assoc_items {
types_count += 1;
*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.get(&item.name).is_some_and(|x| x != &item) {
let trait_def_id = item.container_id(tcx);
shadows = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else {
String::new()
};
let mut is_shadowed = false; let mut names: UnordMap<_, usize> = Default::default();
for item in &missing_assoc_types {
types_count += 1;
*names.entry(item.name).or_insert(0) += 1;
}
let mut dupes = false;
let mut shadows = false;
for item in &missing_assoc_types {
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.get(&item.name).is_some_and(|x| *x != item) {
let trait_def_id = item.container_id(tcx);
shadows = true;
format!("{}::", tcx.def_path_str(trait_def_id))
} else {
String::new()
};
if let Some(assoc_item) = bound_names.get(&item.name) let mut is_shadowed = false;
&& assoc_item != &item
{
is_shadowed = true;
let rename_message = if let Some(assoc_item) = bound_names.get(&item.name)
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" }; && *assoc_item != item
err.span_label(
tcx.def_span(assoc_item.def_id),
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
);
}
let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
err.span_label(
sp,
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
);
}
}
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 let (Ok(snippet), false, false) =
(tcx.sess.source_map().span_to_snippet(*span), dupes, shadows)
{ {
let types: Vec<_> = is_shadowed = true;
assoc_items.iter().map(|item| format!("{} = Type", item.name)).collect();
let code = if snippet.ends_with('>') { let rename_message =
// The user wrote `Trait<'a>` or similar and we don't have a type we can if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
// suggest, but at least we can clue them to the correct syntax err.span_label(
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the tcx.def_span(assoc_item.def_id),
// suggestion. format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", ")) );
} else if in_expr_or_pat { }
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
// least we can clue them to the correct syntax `Iterator::<Item = Type>`. let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
format!("{}::<{}>", snippet, types.join(", "))
} else { if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
// The user wrote `Iterator`, so we don't have a type we can suggest, but at err.span_label(
// least we can clue them to the correct syntax `Iterator<Item = Type>`. sp,
format!("{}<{}>", snippet, types.join(", ")) format!("`{}{}` defined here{}", prefix, item.name, rename_message),
}; );
suggestions.push((*span, code));
} else if dupes {
where_constraints.push(*span);
} }
} }
if potential_assoc_types.len() == missing_assoc_types.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 let (Ok(snippet), false, false) =
(tcx.sess.source_map().span_to_snippet(principal_span), dupes, shadows)
{
let types: Vec<_> =
missing_assoc_types.iter().map(|item| format!("{} = Type", item.name)).collect();
let code = if snippet.ends_with('>') {
// The user wrote `Trait<'a>` or similar and we don't have a type we can
// suggest, but at least we can clue them to the correct syntax
// `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
// suggestion.
format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
} else if in_expr_or_pat {
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
// least we can clue them to the correct syntax `Iterator::<Item = Type>`.
format!("{}::<{}>", snippet, types.join(", "))
} else {
// The user wrote `Iterator`, so we don't have a type we can suggest, but at
// least we can clue them to the correct syntax `Iterator<Item = Type>`.
format!("{}<{}>", snippet, types.join(", "))
};
suggestions.push((principal_span, code));
} else if dupes {
where_constraints.push(principal_span);
}
let where_msg = "consider introducing a new type parameter, adding `where` constraints \ let where_msg = "consider introducing a new type parameter, adding `where` constraints \
using the fully-qualified path to the associated types"; using the fully-qualified path to the associated types";
if !where_constraints.is_empty() && suggestions.is_empty() { if !where_constraints.is_empty() && suggestions.is_empty() {
@ -925,32 +913,30 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
} }
if suggestions.len() != 1 || already_has_generics_args_suggestion { if suggestions.len() != 1 || already_has_generics_args_suggestion {
// We don't need this label if there's an inline suggestion, show otherwise. // We don't need this label if there's an inline suggestion, show otherwise.
for (span, assoc_items) in &associated_types { let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
let mut names: FxIndexMap<_, usize> = FxIndexMap::default(); for item in &missing_assoc_types {
for item in assoc_items { types_count += 1;
types_count += 1; *names.entry(item.name).or_insert(0) += 1;
*names.entry(item.name).or_insert(0) += 1; }
} let mut label = vec![];
let mut label = vec![]; for item in &missing_assoc_types {
for item in assoc_items { let postfix = if names[&item.name] > 1 {
let postfix = if names[&item.name] > 1 { let trait_def_id = item.container_id(tcx);
let trait_def_id = item.container_id(tcx); format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id)) } else {
} else { String::new()
String::new() };
}; label.push(format!("`{}`{}", item.name, postfix));
label.push(format!("`{}`{}", item.name, postfix)); }
} if !label.is_empty() {
if !label.is_empty() { err.span_label(
err.span_label( principal_span,
*span, format!(
format!( "associated type{} {} must be specified",
"associated type{} {} must be specified", pluralize!(label.len()),
pluralize!(label.len()), label.join(", "),
label.join(", "), ),
), );
);
}
} }
} }
suggestions.sort_by_key(|&(span, _)| span); suggestions.sort_by_key(|&(span, _)| span);