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
+184 −163
Merged

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

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
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;
@@ -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()));
@@ -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)) => {
@@ -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,
) {
244 changes: 117 additions & 127 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs
View file
Original file line number Diff line number Diff line change
@@ -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());
}
}

@@ -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}`"),
@@ -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),
@@ -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() {
@@ -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);
4 changes: 2 additions & 2 deletions tests/ui/associated-types/missing-associated-types.stderr
View file
Original file line number Diff line number Diff line change
@@ -42,11 +42,11 @@ LL | type Bat<Rhs> = dyn Add<Rhs> + Sub<Rhs> + Fine<Rhs>;
= help: consider creating a new trait with all of these as supertraits and using that trait here instead: `trait NewTrait: Add<Rhs> + Sub<Rhs> + Fine<Rhs> {}`
= note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit <https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits>

error[E0191]: the value of the associated types `Output` in `Div`, `Output` in `Mul` must be specified
error[E0191]: the value of the associated types `Output` in `Div<Rhs>`, `Output` in `Mul<Rhs>` must be specified
--> $DIR/missing-associated-types.rs:20:21
|
LL | type Bal<Rhs> = dyn X<Rhs>;
| ^^^^^^ associated types `Output` (from trait `Div`), `Output` (from trait `Mul`) must be specified
| ^^^^^^ associated types `Output` (from trait `Div<Rhs>`), `Output` (from trait `Mul<Rhs>`) must be specified
|
= help: consider introducing a new type parameter, adding `where` constraints using the fully-qualified path to the associated types

4 changes: 2 additions & 2 deletions tests/ui/dyn-compatibility/assoc_type_bounds.rs
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ trait Foo<T> {
trait Cake {}
impl Cake for () {}

fn foo(_: &dyn Foo<()>) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified
fn bar(_: &dyn Foo<i32>) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified
fn foo(_: &dyn Foo<()>) {} //~ ERROR: the value of the associated type `Bar` in `Foo<()>` must be specified
fn bar(_: &dyn Foo<i32>) {} //~ ERROR: the value of the associated type `Bar` in `Foo<i32>` must be specified

fn main() {}
4 changes: 2 additions & 2 deletions tests/ui/dyn-compatibility/assoc_type_bounds.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0191]: the value of the associated type `Bar` in `Foo` must be specified
error[E0191]: the value of the associated type `Bar` in `Foo<()>` must be specified
--> $DIR/assoc_type_bounds.rs:10:16
|
LL | type Bar
@@ -7,7 +7,7 @@ LL | type Bar
LL | fn foo(_: &dyn Foo<()>) {}
| ^^^^^^^ help: specify the associated type: `Foo<(), Bar = Type>`

error[E0191]: the value of the associated type `Bar` in `Foo` must be specified
error[E0191]: the value of the associated type `Bar` in `Foo<i32>` must be specified
--> $DIR/assoc_type_bounds.rs:11:16
|
LL | type Bar
4 changes: 2 additions & 2 deletions tests/ui/dyn-compatibility/assoc_type_bounds2.rs
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ trait Foo<T> {
trait Cake {}
impl Cake for () {}

fn foo(_: &dyn Foo<()>) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified
fn bar(_: &dyn Foo<i32>) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified
fn foo(_: &dyn Foo<()>) {} //~ ERROR: the value of the associated type `Bar` in `Foo<()>` must be specified
fn bar(_: &dyn Foo<i32>) {} //~ ERROR: the value of the associated type `Bar` in `Foo<i32>` must be specified

fn main() {}
4 changes: 2 additions & 2 deletions tests/ui/dyn-compatibility/assoc_type_bounds2.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0191]: the value of the associated type `Bar` in `Foo` must be specified
error[E0191]: the value of the associated type `Bar` in `Foo<()>` must be specified
--> $DIR/assoc_type_bounds2.rs:10:16
|
LL | type Bar
@@ -7,7 +7,7 @@ LL | type Bar
LL | fn foo(_: &dyn Foo<()>) {}
| ^^^^^^^ help: specify the associated type: `Foo<(), Bar = Type>`

error[E0191]: the value of the associated type `Bar` in `Foo` must be specified
error[E0191]: the value of the associated type `Bar` in `Foo<i32>` must be specified
--> $DIR/assoc_type_bounds2.rs:11:16
|
LL | type Bar
15 changes: 15 additions & 0 deletions tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
trait Sup<T> {
type Assoc: Default;
}

impl<T: Default> Sup<T> for () {
type Assoc = T;
}
impl<T: Default, U: Default> Dyn<T, U> for () {}

trait Dyn<A, B>: Sup<A, Assoc = A> + Sup<B> {}

fn main() {
let q: <dyn Dyn<i32, u32> as Sup<u32>>::Assoc = Default::default();
//~^ ERROR the value of the associated type `Assoc` in `Sup<u32>` must be specified
}
12 changes: 12 additions & 0 deletions tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0191]: the value of the associated type `Assoc` in `Sup<u32>` must be specified
--> $DIR/require-assoc-for-all-super-substs.rs:13:17
|
LL | type Assoc: Default;
| ------------------- `Assoc` defined here
...
LL | let q: <dyn Dyn<i32, u32> as Sup<u32>>::Assoc = Default::default();
| ^^^^^^^^^^^^^ help: specify the associated type: `Dyn<i32, u32, Assoc = Type>`
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of funny that you just cant specify the associated type equality bounds here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a fundamental limitation of Rust atm. I can't be arsed to improve the diagnostic, though I can open an issue for the diagnostic being confusing if you care.


error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0191`.
4 changes: 2 additions & 2 deletions tests/ui/issues/issue-28344.stderr
View file
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ help: if this is a dyn-compatible trait, use `dyn`
LL | let x: u8 = <dyn BitXor>::bitor(0 as u8, 0 as u8);
| ++++ +

error[E0191]: the value of the associated type `Output` in `BitXor` must be specified
error[E0191]: the value of the associated type `Output` in `BitXor<_>` must be specified
--> $DIR/issue-28344.rs:4:17
|
LL | let x: u8 = BitXor::bitor(0 as u8, 0 as u8);
@@ -31,7 +31,7 @@ help: if this is a dyn-compatible trait, use `dyn`
LL | let g = <dyn BitXor>::bitor;
| ++++ +

error[E0191]: the value of the associated type `Output` in `BitXor` must be specified
error[E0191]: the value of the associated type `Output` in `BitXor<_>` must be specified
--> $DIR/issue-28344.rs:9:13
|
LL | let g = BitXor::bitor;
Loading