Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ICE when multiple supertrait substitutions need assoc but only one is provided #133392

Merged
merged 2 commits into from
Dec 15, 2024
Merged

Fix ICE when multiple supertrait substitutions need assoc but only one is provided #133392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25a7fdf
Dont use span as key when collecting missing associated types from dyn
compiler-errors Nov 23, 2024
43e2fd5
Fix ICE when multiple supertrait substitutions need assoc but only on…
compiler-errors Nov 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 28 additions & 24 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs
View file
Open in desktop
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 @@ -145,13 +145,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let bound_predicate = pred.kind();
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(
tcx.associated_items(pred.def_id())
// FIXME(negative_bounds): Handle this correctly...
let trait_ref =
tcx.anonymize_bound_vars(bound_predicate.rebind(pred.trait_ref));
needed_associated_types.extend(
tcx.associated_items(trait_ref.def_id())
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| !item.is_impl_trait_in_trait())
.map(|item| item.def_id),
// 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, trait_ref)),
);
}
ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred)) => {
Expand Down Expand Up @@ -201,26 +206,25 @@ 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.item_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.item_def_id();
let trait_ref = tcx.anonymize_bound_vars(
projection_bound.map_bound(|p| p.projection_term.trait_ref(tcx)),
);
needed_associated_types.swap_remove(&(def_id, trait_ref));
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
244 changes: 117 additions & 127 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,51 +722,42 @@ 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, ty::PolyTraitRef<'tcx>)>,
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
// FIXME: This logic needs some more care w.r.t handling of conflicts
let missing_assoc_types: Vec<_> = missing_assoc_types
.into_iter()
.map(|(span, def_ids)| {
(span, def_ids.into_iter().map(|did| tcx.associated_item(did)).collect())
})
.map(|(def_id, trait_ref)| (tcx.associated_item(def_id), trait_ref))
.collect();
let mut names: FxIndexMap<String, Vec<Symbol>> = Default::default();
let mut names: FxIndexMap<_, 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, trait_ref) in &missing_assoc_types {
names.entry(trait_ref).or_default().push(assoc_item.name);
names_len += 1;

let violations =
dyn_compatibility_violations_for_assoc_item(tcx, trait_ref.def_id(), *assoc_item);
if !violations.is_empty() {
dyn_compatibility_violations = Err(report_dyn_incompatibility(
tcx,
principal_span,
None,
trait_ref.def_id(),
&violations,
)
.emit());
}
}

Expand Down Expand Up @@ -814,6 +805,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
.into_iter()
.map(|(trait_, mut assocs)| {
assocs.sort();
let trait_ = trait_.print_trait_sugared();
format!("{} in `{trait_}`", match &assocs[..] {
[] => String::new(),
[only] => format!("`{only}`"),
Expand All @@ -827,10 +819,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 @@ -840,81 +831,83 @@ 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, trait_ref) in &missing_assoc_types {
let prefix = if names[&item.name] > 1 {
let trait_def_id = trait_ref.def_id();
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 = trait_ref.def_id();
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 @@ -925,32 +918,29 @@ 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, trait_ref) in &missing_assoc_types {
let postfix = if names[&item.name] > 1 {
format!(" (from trait `{}`)", trait_ref.print_trait_sugared())
} 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
Loading
Loading