From 08135254dcf22be0d5661ea8f75e703b29a83514 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 2 Jul 2022 01:30:07 +0000 Subject: [PATCH] Highlight conflicting param-env candidates --- compiler/rustc_middle/src/traits/mod.rs | 10 ++++- .../src/traits/error_reporting/mod.rs | 41 +++++++++++++----- .../src/traits/select/candidate_assembly.rs | 43 ++++++++++++++++++- src/test/ui/issues/issue-21974.stderr | 8 +++- src/test/ui/issues/issue-24424.stderr | 6 ++- src/test/ui/lifetimes/issue-34979.stderr | 8 +++- src/test/ui/traits/issue-85735.stderr | 9 +++- .../ui/type/type-check/issue-40294.stderr | 8 +++- 8 files changed, 114 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index eee44df8645ef..fe7f72024d358 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -546,9 +546,17 @@ pub enum SelectionError<'tcx> { ErrorReporting, /// Multiple applicable `impl`s where found. The `DefId`s correspond to /// all the `impl`s' Items. - Ambiguous(Vec), + Ambiguous(Vec), } +#[derive(Copy, Clone, Debug)] +pub enum AmbiguousSelection { + Impl(DefId), + ParamEnv(Span), +} + +TrivialTypeTraversalAndLiftImpls! { AmbiguousSelection, } + /// When performing resolution, it is typically the case that there /// can be one of three outcomes: /// 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 8efefd476abc9..aa1c91362891b 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -23,7 +23,7 @@ use rustc_hir::GenericParam; use rustc_hir::Item; use rustc_hir::Node; use rustc_infer::infer::error_reporting::same_type_modulo_infer; -use rustc_infer::traits::TraitEngine; +use rustc_infer::traits::{AmbiguousSelection, TraitEngine}; use rustc_middle::thir::abstract_const::NotConstEvaluatable; use rustc_middle::traits::select::OverflowError; use rustc_middle::ty::error::ExpectedFound; @@ -1404,7 +1404,7 @@ trait InferCtxtPrivExt<'hir, 'tcx> { fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + impls: &[AmbiguousSelection], predicate: ty::Predicate<'tcx>, ); @@ -2020,6 +2020,14 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { ); match selcx.select_from_obligation(&obligation) { Err(SelectionError::Ambiguous(impls)) if impls.len() > 1 => { + if self.is_tainted_by_errors() && 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, &impls, predicate); } _ => { @@ -2170,24 +2178,35 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { fn annotate_source_of_ambiguity( &self, err: &mut Diagnostic, - impls: &[DefId], + impls: &[AmbiguousSelection], 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 or_where_clause = false; + for ambig in impls { + match ambig { + AmbiguousSelection::Impl(def_id) => 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); + } } + }, + AmbiguousSelection::ParamEnv(span) => { + or_where_clause = true; + spans.push(*span); } } } - let msg = format!("multiple `impl`s satisfying `{}` found", predicate); + let msg = format!( + "multiple `impl`s{} satisfying `{}` found", + if or_where_clause { " or `where` clauses" } else { "" }, + predicate + ); let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{}`", n)).collect(); crate_names.sort(); crate_names.dedup(); diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 0f7af41cfe3ff..21e14eae0ee27 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -6,9 +6,11 @@ //! //! [rustc dev guide]:https://rustc-dev-guide.rust-lang.org/traits/resolution.html#candidate-assembly use hir::LangItem; +use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_hir::def_id::DefId; -use rustc_infer::traits::TraitEngine; +use rustc_infer::traits::util::elaborate_predicates_with_span; +use rustc_infer::traits::{AmbiguousSelection, TraitEngine}; use rustc_infer::traits::{Obligation, SelectionError, TraitObligation}; use rustc_lint_defs::builtin::DEREF_INTO_DYN_SUPERTRAIT; use rustc_middle::ty::print::with_no_trimmed_paths; @@ -199,11 +201,48 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // and report ambiguity. if i > 1 { debug!("multiple matches, ambig"); + + // Collect a list of (probable) spans that point to a param-env candidate + let tcx = self.infcx.tcx; + let owner = stack.obligation.cause.body_id.owner.to_def_id(); + let predicates = tcx.predicates_of(owner).instantiate_identity(tcx); + let param_env_spans: FxHashMap<_, _> = elaborate_predicates_with_span( + tcx, + std::iter::zip(predicates.predicates, predicates.spans), + ) + .filter_map(|obligation| { + let kind = obligation.predicate.kind(); + if let ty::PredicateKind::Trait(trait_pred) = kind.skip_binder() { + if trait_pred.trait_ref + == ty::TraitRef::identity(tcx, trait_pred.def_id()) + .skip_binder() + { + // HACK: Remap the `Self: Trait` predicate that every trait has to a more useful span + Some(( + kind.rebind(trait_pred), + tcx.def_span(trait_pred.def_id()), + )) + } else { + Some((kind.rebind(trait_pred), obligation.cause.span)) + } + } else { + None + } + }) + .collect(); + return Err(Ambiguous( candidates .into_iter() .filter_map(|c| match c.candidate { - SelectionCandidate::ImplCandidate(def_id) => Some(def_id), + SelectionCandidate::ImplCandidate(def_id) => { + Some(AmbiguousSelection::Impl(def_id)) + } + SelectionCandidate::ParamCandidate(predicate) => { + Some(AmbiguousSelection::ParamEnv( + *param_env_spans.get(&predicate)?, + )) + } _ => None, }) .collect(), diff --git a/src/test/ui/issues/issue-21974.stderr b/src/test/ui/issues/issue-21974.stderr index 4e010a13653e7..2d60b18b1f208 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 8f3b2ac73199c..50d7f988e194c 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 5832c4d173c10..a78b3eaf6258d 100644 --- a/src/test/ui/lifetimes/issue-34979.stderr +++ b/src/test/ui/lifetimes/issue-34979.stderr @@ -4,7 +4,13 @@ error[E0283]: type annotations needed: cannot satisfy `&'a (): Foo` LL | &'a (): Foo, | ^^^ | - = note: cannot satisfy `&'a (): Foo` +note: multiple `impl`s or `where` clauses satisfying `&'a (): Foo` found + --> $DIR/issue-34979.rs:6:13 + | +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 fa280135beb2d..9e80497ca6e92 100644 --- a/src/test/ui/traits/issue-85735.stderr +++ b/src/test/ui/traits/issue-85735.stderr @@ -4,7 +4,14 @@ error[E0283]: type annotations needed: cannot satisfy `T: FnMut<(&'a (),)>` LL | T: FnMut(&'a ()), | ^^^^^^^^^^^^^ | - = note: cannot satisfy `T: FnMut<(&'a (),)>` +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 75feb5698eb63..d15fd23418bb0 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