Skip to content

Commit

Permalink
Dont use span as key when collecting missing associated types from dyn
Browse files Browse the repository at this point in the history
  • Loading branch information
@compiler-errors
compiler-errors committed Dec 4, 2024
1 parent 96e51d9 commit 5469673
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 149 deletions.
41 changes: 20 additions & 21 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_errors::codes::*;
use rustc_errors::struct_span_code_err;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
use rustc_middle::span_bug;
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{
self, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
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::traits::{self, hir_ty_lowering_dyn_compatibility_violations};
use rustc_type_ir::elaborate::ClauseWithSupertraitSpan;
Expand Down Expand Up @@ -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
.into_iter()
.filter(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id()));
Expand All @@ -146,11 +146,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
match bound_predicate.skip_binder() {
ty::PredicateKind::Clause(ty::ClauseKind::Trait(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())
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.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),
);
}
Expand Down Expand Up @@ -201,26 +204,22 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// 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
// corresponding `Projection` clause
for def_ids in associated_types.values_mut() {
for (projection_bound, span) in &projection_bounds {
let def_id = projection_bound.projection_def_id();
def_ids.swap_remove(&def_id);
if tcx.generics_require_sized_self(def_id) {
tcx.emit_node_span_lint(
UNUSED_ASSOCIATED_TYPE_BOUNDS,
hir_id,
*span,
crate::errors::UnusedAssociatedTypeBounds { span: *span },
);
}
for (projection_bound, span) in &projection_bounds {
let def_id = projection_bound.projection_def_id();
needed_associated_types.swap_remove(&def_id);
if tcx.generics_require_sized_self(def_id) {
tcx.emit_node_span_lint(
UNUSED_ASSOCIATED_TYPE_BOUNDS,
hir_id,
*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(
associated_types,
principal_span,
needed_associated_types,
potential_assoc_types,
hir_trait_bounds,
) {
Expand Down
242 changes: 114 additions & 128 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -716,51 +716,40 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
/// emit a generic note suggesting using a `where` clause to constraint instead.
pub(crate) fn check_for_required_assoc_tys(
&self,
associated_types: FxIndexMap<Span, FxIndexSet<DefId>>,
principal_span: Span,
missing_assoc_types: FxIndexSet<DefId>,
potential_assoc_types: Vec<usize>,
trait_bounds: &[hir::PolyTraitRef<'_>],
) -> Result<(), ErrorGuaranteed> {
if associated_types.values().all(|v| v.is_empty()) {
if missing_assoc_types.is_empty() {
return Ok(());
}

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.
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 missing_assoc_types: Vec<_> =
missing_assoc_types.into_iter().map(|def_id| tcx.associated_item(def_id)).collect();
let mut names: FxIndexMap<String, Vec<Symbol>> = Default::default();
let mut names_len = 0;

// Account for things like `dyn Foo + 'a`, like in tests `issue-22434.rs` and
// `issue-22560.rs`.
let mut trait_bound_spans: Vec<Span> = vec![];
let mut dyn_compatibility_violations = Ok(());
for (span, items) in &associated_types {
if !items.is_empty() {
trait_bound_spans.push(*span);
}
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 =
dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item);
if !violations.is_empty() {
dyn_compatibility_violations = Err(report_dyn_incompatibility(
tcx,
*span,
None,
trait_def_id,
&violations,
)
.emit());
}
for assoc_item in &missing_assoc_types {
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 =
dyn_compatibility_violations_for_assoc_item(tcx, trait_def_id, *assoc_item);
if !violations.is_empty() {
dyn_compatibility_violations = Err(report_dyn_incompatibility(
tcx,
principal_span,
None,
trait_def_id,
&violations,
)
.emit());
}
}

Expand Down Expand Up @@ -821,10 +810,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
names.sort();
let names = names.join(", ");

trait_bound_spans.sort();
let mut err = struct_span_code_err!(
self.dcx(),
trait_bound_spans,
principal_span,
E0191,
"the value of the associated type{} {} must be specified",
pluralize!(names_len),
Expand All @@ -834,81 +822,81 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let mut types_count = 0;
let mut where_constraints = vec![];
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;

if let Some(assoc_item) = bound_names.get(&item.name)
&& assoc_item != &item
{
is_shadowed = true;
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()
};

let rename_message =
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
err.span_label(
tcx.def_span(assoc_item.def_id),
format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
);
}
let mut is_shadowed = false;

let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
if let Some(assoc_item) = bound_names.get(&item.name)
&& *assoc_item != item
{
is_shadowed = true;

if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
err.span_label(
sp,
format!("`{}{}` defined here{}", prefix, item.name, rename_message),
);
}
let rename_message =
if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
err.span_label(
tcx.def_span(assoc_item.def_id),
format!("`{}{}` shadowed 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<_> =
assoc_items.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((*span, code));
} else if dupes {
where_constraints.push(*span);

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() == 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 \
using the fully-qualified path to the associated types";
if !where_constraints.is_empty() && suggestions.is_empty() {
Expand All @@ -919,32 +907,30 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}
if suggestions.len() != 1 || already_has_generics_args_suggestion {
// 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();
for item in assoc_items {
types_count += 1;
*names.entry(item.name).or_insert(0) += 1;
}
let mut label = vec![];
for item in assoc_items {
let postfix = if names[&item.name] > 1 {
let trait_def_id = item.container_id(tcx);
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
} else {
String::new()
};
label.push(format!("`{}`{}", item.name, postfix));
}
if !label.is_empty() {
err.span_label(
*span,
format!(
"associated type{} {} must be specified",
pluralize!(label.len()),
label.join(", "),
),
);
}
let mut names: FxIndexMap<_, usize> = FxIndexMap::default();
for item in &missing_assoc_types {
types_count += 1;
*names.entry(item.name).or_insert(0) += 1;
}
let mut label = vec![];
for item in &missing_assoc_types {
let postfix = if names[&item.name] > 1 {
let trait_def_id = item.container_id(tcx);
format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
} else {
String::new()
};
label.push(format!("`{}`{}", item.name, postfix));
}
if !label.is_empty() {
err.span_label(
principal_span,
format!(
"associated type{} {} must be specified",
pluralize!(label.len()),
label.join(", "),
),
);
}
}
suggestions.sort_by_key(|&(span, _)| span);
Expand Down

0 comments on commit 5469673

Please sign in to comment.