From 24da0538f48d7ac9c5f90c7d09472e3e2200575c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 23 Nov 2024 18:46:50 +0000 Subject: [PATCH] Fix ICE when multiple supertrait substitutions need assoc but only one is provided --- .../src/hir_ty_lowering/dyn_compatibility.rs | 13 ++++-- .../src/hir_ty_lowering/errors.rs | 42 ++++++++++--------- .../missing-associated-types.stderr | 4 +- .../ui/dyn-compatibility/assoc_type_bounds.rs | 4 +- .../assoc_type_bounds.stderr | 4 +- .../dyn-compatibility/assoc_type_bounds2.rs | 4 +- .../assoc_type_bounds2.stderr | 4 +- .../require-assoc-for-all-super-substs.rs | 15 +++++++ .../require-assoc-for-all-super-substs.stderr | 12 ++++++ tests/ui/issues/issue-28344.stderr | 4 +- 10 files changed, 71 insertions(+), 35 deletions(-) create mode 100644 tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.rs create mode 100644 tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.stderr diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs index 470037d31e69f..a13c127d03f82 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs @@ -145,16 +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); + // 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(pred.def_id()) + 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()) // 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, trait_ref)), ); } ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred)) => { @@ -206,7 +208,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // corresponding `Projection` clause for (projection_bound, span) in &projection_bounds { let def_id = projection_bound.projection_def_id(); - needed_associated_types.swap_remove(&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, diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs index 4e7747173701d..622737d51a9e0 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs @@ -717,7 +717,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { pub(crate) fn check_for_required_assoc_tys( &self, principal_span: Span, - missing_assoc_types: FxIndexSet, + missing_assoc_types: FxIndexSet<(DefId, ty::PolyTraitRef<'tcx>)>, potential_assoc_types: Vec, trait_bounds: &[hir::PolyTraitRef<'_>], ) -> Result<(), ErrorGuaranteed> { @@ -726,27 +726,29 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } let tcx = self.tcx(); - let missing_assoc_types: Vec<_> = - missing_assoc_types.into_iter().map(|def_id| tcx.associated_item(def_id)).collect(); - let mut names: FxIndexMap> = Default::default(); + // FIXME: This logic needs some more care w.r.t handling of conflicts + let missing_assoc_types: Vec<_> = missing_assoc_types + .into_iter() + .map(|(def_id, trait_ref)| (tcx.associated_item(def_id), trait_ref)) + .collect(); + let mut names: FxIndexMap<_, Vec> = 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 dyn_compatibility_violations = Ok(()); - 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); + 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_def_id, *assoc_item); + 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_def_id, + trait_ref.def_id(), &violations, ) .emit()); @@ -797,6 +799,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}`"), @@ -824,19 +827,19 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { let mut already_has_generics_args_suggestion = false; let mut names: UnordMap<_, usize> = Default::default(); - for item in &missing_assoc_types { + 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 { + for (item, trait_ref) in &missing_assoc_types { let prefix = if names[&item.name] > 1 { - let trait_def_id = item.container_id(tcx); + 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 = item.container_id(tcx); + let trait_def_id = trait_ref.def_id(); shadows = true; format!("{}::", tcx.def_path_str(trait_def_id)) } else { @@ -875,8 +878,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } 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 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 @@ -908,15 +913,14 @@ 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. let mut names: FxIndexMap<_, usize> = FxIndexMap::default(); - for item in &missing_assoc_types { + 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 { + for (item, trait_ref) 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)) + format!(" (from trait `{}`)", trait_ref.print_trait_sugared()) } else { String::new() }; diff --git a/tests/ui/associated-types/missing-associated-types.stderr b/tests/ui/associated-types/missing-associated-types.stderr index ce4b57e8af813..3a56c55896ebf 100644 --- a/tests/ui/associated-types/missing-associated-types.stderr +++ b/tests/ui/associated-types/missing-associated-types.stderr @@ -42,11 +42,11 @@ LL | type Bat = dyn Add + Sub + Fine; = help: consider creating a new trait with all of these as supertraits and using that trait here instead: `trait NewTrait: Add + Sub + Fine {}` = note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit -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`, `Output` in `Mul` must be specified --> $DIR/missing-associated-types.rs:20:21 | LL | type Bal = dyn X; - | ^^^^^^ associated types `Output` (from trait `Div`), `Output` (from trait `Mul`) must be specified + | ^^^^^^ associated types `Output` (from trait `Div`), `Output` (from trait `Mul`) must be specified | = help: consider introducing a new type parameter, adding `where` constraints using the fully-qualified path to the associated types diff --git a/tests/ui/dyn-compatibility/assoc_type_bounds.rs b/tests/ui/dyn-compatibility/assoc_type_bounds.rs index 8634ba626a18a..6e2076a482247 100644 --- a/tests/ui/dyn-compatibility/assoc_type_bounds.rs +++ b/tests/ui/dyn-compatibility/assoc_type_bounds.rs @@ -7,7 +7,7 @@ trait Foo { 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) {} //~ 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) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified fn main() {} diff --git a/tests/ui/dyn-compatibility/assoc_type_bounds.stderr b/tests/ui/dyn-compatibility/assoc_type_bounds.stderr index 3d5482625af3e..21ba903011739 100644 --- a/tests/ui/dyn-compatibility/assoc_type_bounds.stderr +++ b/tests/ui/dyn-compatibility/assoc_type_bounds.stderr @@ -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` must be specified --> $DIR/assoc_type_bounds.rs:11:16 | LL | type Bar diff --git a/tests/ui/dyn-compatibility/assoc_type_bounds2.rs b/tests/ui/dyn-compatibility/assoc_type_bounds2.rs index f7dc2fb88390f..2b35016d774ce 100644 --- a/tests/ui/dyn-compatibility/assoc_type_bounds2.rs +++ b/tests/ui/dyn-compatibility/assoc_type_bounds2.rs @@ -7,7 +7,7 @@ trait Foo { 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) {} //~ 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) {} //~ ERROR: the value of the associated type `Bar` in `Foo` must be specified fn main() {} diff --git a/tests/ui/dyn-compatibility/assoc_type_bounds2.stderr b/tests/ui/dyn-compatibility/assoc_type_bounds2.stderr index 815747436bf0d..5c4163b196936 100644 --- a/tests/ui/dyn-compatibility/assoc_type_bounds2.stderr +++ b/tests/ui/dyn-compatibility/assoc_type_bounds2.stderr @@ -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` must be specified --> $DIR/assoc_type_bounds2.rs:11:16 | LL | type Bar diff --git a/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.rs b/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.rs new file mode 100644 index 0000000000000..1f4e1bf653a3f --- /dev/null +++ b/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.rs @@ -0,0 +1,15 @@ +trait Sup { + type Assoc: Default; +} + +impl Sup for () { + type Assoc = T; +} +impl Dyn for () {} + +trait Dyn: Sup + Sup {} + +fn main() { + let q: as Sup>::Assoc = Default::default(); + //~^ ERROR the value of the associated type `Assoc` in `Sup` must be specified +} diff --git a/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.stderr b/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.stderr new file mode 100644 index 0000000000000..3d89b52d522db --- /dev/null +++ b/tests/ui/dyn-compatibility/require-assoc-for-all-super-substs.stderr @@ -0,0 +1,12 @@ +error[E0191]: the value of the associated type `Assoc` in `Sup` must be specified + --> $DIR/require-assoc-for-all-super-substs.rs:13:17 + | +LL | type Assoc: Default; + | ------------------- `Assoc` defined here +... +LL | let q: as Sup>::Assoc = Default::default(); + | ^^^^^^^^^^^^^ help: specify the associated type: `Dyn` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0191`. diff --git a/tests/ui/issues/issue-28344.stderr b/tests/ui/issues/issue-28344.stderr index d8febe7165241..7bc965536e9a1 100644 --- a/tests/ui/issues/issue-28344.stderr +++ b/tests/ui/issues/issue-28344.stderr @@ -12,7 +12,7 @@ help: if this is a dyn-compatible trait, use `dyn` LL | let x: u8 = ::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 = ::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;