From 34d194d41c8ffea79b8816a811b2fe626b1a6c39 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 15 Nov 2022 23:11:11 +0000 Subject: [PATCH] Highlight conflicting param-env candidates, again --- .../src/traits/error_reporting/ambiguity.rs | 81 +++++++++++++++---- .../src/traits/error_reporting/mod.rs | 48 ++++++++--- .../issue-72787.min.stderr | 26 +++++- src/test/ui/issues/issue-21974.stderr | 8 +- src/test/ui/issues/issue-24424.stderr | 6 +- src/test/ui/lifetimes/issue-34979.stderr | 12 ++- src/test/ui/traits/issue-85735.stderr | 12 ++- .../ui/type/type-check/issue-40294.stderr | 8 +- 8 files changed, 159 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/ambiguity.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/ambiguity.rs index 752b53fbc3f..0c1717cff33 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/ambiguity.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/ambiguity.rs @@ -1,52 +1,101 @@ use rustc_hir::def_id::DefId; -use rustc_infer::infer::InferCtxt; +use rustc_infer::infer::{InferCtxt, LateBoundRegionConversionTime}; +use rustc_infer::traits::util::elaborate_predicates_with_span; use rustc_infer::traits::{Obligation, ObligationCause, TraitObligation}; -use rustc_span::DUMMY_SP; +use rustc_middle::ty; +use rustc_span::{Span, DUMMY_SP}; use crate::traits::ObligationCtxt; +pub enum Ambiguity { + DefId(DefId), + ParamEnv(Span), +} + pub fn recompute_applicable_impls<'tcx>( infcx: &InferCtxt<'tcx>, obligation: &TraitObligation<'tcx>, -) -> Vec { +) -> Vec { let tcx = infcx.tcx; let param_env = obligation.param_env; - let dummy_cause = ObligationCause::dummy(); + let impl_may_apply = |impl_def_id| { let ocx = ObligationCtxt::new_in_snapshot(infcx); let placeholder_obligation = infcx.replace_bound_vars_with_placeholders(obligation.predicate); let obligation_trait_ref = - ocx.normalize(&dummy_cause, param_env, placeholder_obligation.trait_ref); + ocx.normalize(&ObligationCause::dummy(), param_env, placeholder_obligation.trait_ref); let impl_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id); let impl_trait_ref = tcx.bound_impl_trait_ref(impl_def_id).unwrap().subst(tcx, impl_substs); let impl_trait_ref = ocx.normalize(&ObligationCause::dummy(), param_env, impl_trait_ref); - if let Err(_) = ocx.eq(&dummy_cause, param_env, obligation_trait_ref, impl_trait_ref) { + if let Err(_) = + ocx.eq(&ObligationCause::dummy(), param_env, obligation_trait_ref, impl_trait_ref) + { return false; } let impl_predicates = tcx.predicates_of(impl_def_id).instantiate(tcx, impl_substs); - ocx.register_obligations( - impl_predicates - .predicates - .iter() - .map(|&predicate| Obligation::new(tcx, dummy_cause.clone(), param_env, predicate)), - ); + ocx.register_obligations(impl_predicates.predicates.iter().map(|&predicate| { + Obligation::new(tcx, ObligationCause::dummy(), param_env, predicate) + })); ocx.select_where_possible().is_empty() }; - let mut impls = Vec::new(); + let param_env_candidate_may_apply = |poly_trait_predicate: ty::PolyTraitPredicate<'tcx>| { + let ocx = ObligationCtxt::new_in_snapshot(infcx); + let placeholder_obligation = + infcx.replace_bound_vars_with_placeholders(obligation.predicate); + let obligation_trait_ref = + ocx.normalize(&ObligationCause::dummy(), param_env, placeholder_obligation.trait_ref); + + let param_env_predicate = infcx.replace_bound_vars_with_fresh_vars( + DUMMY_SP, + LateBoundRegionConversionTime::HigherRankedType, + poly_trait_predicate, + ); + let param_env_trait_ref = + ocx.normalize(&ObligationCause::dummy(), param_env, param_env_predicate.trait_ref); + + if let Err(_) = + ocx.eq(&ObligationCause::dummy(), param_env, obligation_trait_ref, param_env_trait_ref) + { + return false; + } + + ocx.select_where_possible().is_empty() + }; + + let mut ambiguities = Vec::new(); + tcx.for_each_relevant_impl( obligation.predicate.def_id(), obligation.predicate.skip_binder().trait_ref.self_ty(), |impl_def_id| { - if infcx.probe(move |_snapshot| impl_may_apply(impl_def_id)) { - impls.push(impl_def_id) + if infcx.probe(|_| impl_may_apply(impl_def_id)) { + ambiguities.push(Ambiguity::DefId(impl_def_id)) } }, ); - impls + + let predicates = + tcx.predicates_of(obligation.cause.body_id.owner.to_def_id()).instantiate_identity(tcx); + for obligation in + elaborate_predicates_with_span(tcx, std::iter::zip(predicates.predicates, predicates.spans)) + { + let kind = obligation.predicate.kind(); + if let ty::PredicateKind::Clause(ty::Clause::Trait(trait_pred)) = kind.skip_binder() + && param_env_candidate_may_apply(kind.rebind(trait_pred)) + { + if kind.rebind(trait_pred.trait_ref) == ty::TraitRef::identity(tcx, trait_pred.def_id()) { + ambiguities.push(Ambiguity::ParamEnv(tcx.def_span(trait_pred.def_id()))) + } else { + ambiguities.push(Ambiguity::ParamEnv(obligation.cause.span)) + } + } + } + + ambiguities } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index c68da3e24a1..dcbc2b332c2 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1487,7 +1487,7 @@ fn suggest_unsized_bound_if_applicable( fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + impls: &[ambiguity::Ambiguity], predicate: ty::Predicate<'tcx>, ); @@ -2180,13 +2180,22 @@ fn maybe_report_ambiguity( let mut selcx = SelectionContext::new(&self); match selcx.select_from_obligation(&obligation) { Ok(None) => { - let impls = ambiguity::recompute_applicable_impls(self.infcx, &obligation); + let ambiguities = + ambiguity::recompute_applicable_impls(self.infcx, &obligation); let has_non_region_infer = trait_ref.skip_binder().substs.types().any(|t| !t.is_ty_infer()); // It doesn't make sense to talk about applicable impls if there are more // than a handful of them. - if impls.len() > 1 && impls.len() < 10 && has_non_region_infer { - self.annotate_source_of_ambiguity(&mut err, &impls, predicate); + if ambiguities.len() > 1 && ambiguities.len() < 10 && has_non_region_infer { + if self.tainted_by_errors().is_some() && subst.is_none() { + // If `subst.is_none()`, then this is probably two param-env + // candidates or impl candidates that are equal modulo lifetimes. + // Therefore, if we've already emitted an error, just skip this + // one, since it's not particularly actionable. + err.cancel(); + return; + } + self.annotate_source_of_ambiguity(&mut err, &ambiguities, predicate); } else { if self.tainted_by_errors().is_some() { err.cancel(); @@ -2434,21 +2443,30 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + ambiguities: &[ambiguity::Ambiguity], predicate: ty::Predicate<'tcx>, ) { let mut spans = vec![]; let mut crates = vec![]; let mut post = vec![]; - for def_id in impls { - match self.tcx.span_of_impl(*def_id) { - Ok(span) => spans.push(span), - Err(name) => { - crates.push(name); - if let Some(header) = to_pretty_impl_header(self.tcx, *def_id) { - post.push(header); + let mut has_param_env = false; + for ambiguity in ambiguities { + match ambiguity { + ambiguity::Ambiguity::DefId(impl_def_id) => { + match self.tcx.span_of_impl(*impl_def_id) { + Ok(span) => spans.push(span), + Err(name) => { + crates.push(name); + if let Some(header) = to_pretty_impl_header(self.tcx, *impl_def_id) { + post.push(header); + } + } } } + ambiguity::Ambiguity::ParamEnv(span) => { + has_param_env = true; + spans.push(*span); + } } } let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{}`", n)).collect(); @@ -2472,7 +2490,11 @@ fn annotate_source_of_ambiguity( return; } - let msg = format!("multiple `impl`s satisfying `{}` found", predicate); + let msg = format!( + "multiple `impl`s{} satisfying `{}` found", + if has_param_env { " or `where` clauses" } else { "" }, + predicate + ); let post = if post.len() > 1 || (post.len() == 1 && post[0].contains('\n')) { format!(":\n{}", post.iter().map(|p| format!("- {}", p)).collect::>().join("\n"),) } else if post.len() == 1 { diff --git a/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr b/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr index e0444042614..0af5493f816 100644 --- a/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr +++ b/src/test/ui/const-generics/generic_const_exprs/issue-72787.min.stderr @@ -40,8 +40,17 @@ error[E0283]: type annotations needed: cannot satisfy `IsLessOrEqual: True LL | IsLessOrEqual: True, | ^^^^ | - = note: cannot satisfy `IsLessOrEqual: True` - = help: the trait `True` is implemented for `IsLessOrEqual` +note: multiple `impl`s or `where` clauses satisfying `IsLessOrEqual: True` found + --> $DIR/issue-72787.rs:10:1 + | +LL | impl True for IsLessOrEqual where + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | IsLessOrEqual: True, + | ^^^^ +... +LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, + | ^^^^ error[E0283]: type annotations needed: cannot satisfy `IsLessOrEqual: True` --> $DIR/issue-72787.rs:21:26 @@ -49,8 +58,17 @@ error[E0283]: type annotations needed: cannot satisfy `IsLessOrEqual: True LL | IsLessOrEqual: True, | ^^^^ | - = note: cannot satisfy `IsLessOrEqual: True` - = help: the trait `True` is implemented for `IsLessOrEqual` +note: multiple `impl`s or `where` clauses satisfying `IsLessOrEqual: True` found + --> $DIR/issue-72787.rs:10:1 + | +LL | impl True for IsLessOrEqual where + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | IsLessOrEqual: True, + | ^^^^ +... +LL | IsLessOrEqual<{ 8 - I }, { 8 - J }>: True, + | ^^^^ error: aborting due to 6 previous errors diff --git a/src/test/ui/issues/issue-21974.stderr b/src/test/ui/issues/issue-21974.stderr index 4e010a13653..2d60b18b1f2 100644 --- a/src/test/ui/issues/issue-21974.stderr +++ b/src/test/ui/issues/issue-21974.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a T: Foo` LL | where &'a T : Foo, | ^^^ | - = note: cannot satisfy `&'a T: Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a T: Foo` found + --> $DIR/issue-21974.rs:11:19 + | +LL | where &'a T : Foo, + | ^^^ +LL | &'b T : Foo + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-24424.stderr b/src/test/ui/issues/issue-24424.stderr index 8f3b2ac7319..50d7f988e19 100644 --- a/src/test/ui/issues/issue-24424.stderr +++ b/src/test/ui/issues/issue-24424.stderr @@ -4,7 +4,11 @@ error[E0283]: type annotations needed: cannot satisfy `T0: Trait0<'l0>` LL | impl <'l0, 'l1, T0> Trait1<'l0, T0> for bool where T0 : Trait0<'l0>, T0 : Trait0<'l1> {} | ^^^^^^^^^^^ | - = note: cannot satisfy `T0: Trait0<'l0>` +note: multiple `impl`s or `where` clauses satisfying `T0: Trait0<'l0>` found + --> $DIR/issue-24424.rs:4:57 + | +LL | impl <'l0, 'l1, T0> Trait1<'l0, T0> for bool where T0 : Trait0<'l0>, T0 : Trait0<'l1> {} + | ^^^^^^^^^^^ ^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/lifetimes/issue-34979.stderr b/src/test/ui/lifetimes/issue-34979.stderr index a332c6547b8..3d4208031cd 100644 --- a/src/test/ui/lifetimes/issue-34979.stderr +++ b/src/test/ui/lifetimes/issue-34979.stderr @@ -4,8 +4,16 @@ error[E0283]: type annotations needed: cannot satisfy `&'a (): Foo` LL | &'a (): Foo, | ^^^ | - = note: cannot satisfy `&'a (): Foo` - = help: the trait `Foo` is implemented for `&'a T` +note: multiple `impl`s or `where` clauses satisfying `&'a (): Foo` found + --> $DIR/issue-34979.rs:2:1 + | +LL | impl<'a, T> Foo for &'a T {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | &'a (): Foo, + | ^^^ +LL | &'static (): Foo; + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/traits/issue-85735.stderr b/src/test/ui/traits/issue-85735.stderr index 930708f9ad8..9e80497ca6e 100644 --- a/src/test/ui/traits/issue-85735.stderr +++ b/src/test/ui/traits/issue-85735.stderr @@ -4,10 +4,14 @@ error[E0283]: type annotations needed: cannot satisfy `T: FnMut<(&'a (),)>` LL | T: FnMut(&'a ()), | ^^^^^^^^^^^^^ | - = note: cannot satisfy `T: FnMut<(&'a (),)>` - = help: the following types implement trait `FnMut`: - &F - &mut F +note: multiple `impl`s or `where` clauses satisfying `T: FnMut<(&'a (),)>` found + --> $DIR/issue-85735.rs:7:8 + | +LL | T: FnMut(&'a ()), + | ^^^^^^^^^^^^^ +LL | +LL | T: FnMut(&'b ()), + | ^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/type/type-check/issue-40294.stderr b/src/test/ui/type/type-check/issue-40294.stderr index 75feb5698eb..d15fd23418b 100644 --- a/src/test/ui/type/type-check/issue-40294.stderr +++ b/src/test/ui/type/type-check/issue-40294.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a T: Foo` LL | where &'a T : Foo, | ^^^ | - = note: cannot satisfy `&'a T: Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a T: Foo` found + --> $DIR/issue-40294.rs:6:19 + | +LL | where &'a T : Foo, + | ^^^ +LL | &'b T : Foo + | ^^^ error: aborting due to previous error