From db235a07f79a34fe9e40bb122d6ae78f05bc2623 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 20 Jun 2023 20:00:39 +0000 Subject: [PATCH 1/3] Remove unnecessary call to select_from_obligation The only regression is one ambiguity in the new trait solver, having to do with two param-env candidates that may apply. I think this is fine, since the error message already kinda sucks. --- .../src/traits/error_reporting/mod.rs | 83 ++++++++----------- ...tion-param-candidates-are-ambiguous.stderr | 1 + 2 files changed, 35 insertions(+), 49 deletions(-) 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 398ec28a42638..75aee0670a258 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -5,7 +5,7 @@ pub mod suggestions; use super::{ FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode, ObligationCtxt, OutputTypeParameterMismatch, Overflow, - PredicateObligation, SelectionContext, SelectionError, TraitNotObjectSafe, + PredicateObligation, SelectionError, TraitNotObjectSafe, }; use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode}; use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; @@ -2269,55 +2269,40 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { ) }; - let obligation = obligation.with(self.tcx, trait_ref); - let mut selcx = SelectionContext::new(&self); - match selcx.select_from_obligation(&obligation) { - Ok(None) => { - 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_or_numeric_infer()); - // It doesn't make sense to talk about applicable impls if there are more - // than a handful of them. - 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(); - return; - } - err.note(format!("cannot satisfy `{}`", predicate)); - let impl_candidates = self.find_similar_impl_candidates( - predicate.to_opt_poly_trait_pred().unwrap(), - ); - if impl_candidates.len() < 10 { - self.report_similar_impl_candidates( - impl_candidates.as_slice(), - trait_ref, - obligation.cause.body_id, - &mut err, - false, - ); - } - } + let ambiguities = ambiguity::recompute_applicable_impls( + self.infcx, + &obligation.with(self.tcx, trait_ref), + ); + let has_non_region_infer = + trait_ref.skip_binder().substs.types().any(|t| !t.is_ty_or_numeric_infer()); + // It doesn't make sense to talk about applicable impls if there are more + // than a handful of them. + 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; } - _ => { - if self.tainted_by_errors().is_some() { - err.cancel(); - return; - } - err.note(format!("cannot satisfy `{}`", predicate)); + self.annotate_source_of_ambiguity(&mut err, &ambiguities, predicate); + } else { + if self.tainted_by_errors().is_some() { + err.cancel(); + return; + } + err.note(format!("cannot satisfy `{}`", predicate)); + let impl_candidates = self + .find_similar_impl_candidates(predicate.to_opt_poly_trait_pred().unwrap()); + if impl_candidates.len() < 10 { + self.report_similar_impl_candidates( + impl_candidates.as_slice(), + trait_ref, + obligation.cause.body_id, + &mut err, + false, + ); } } diff --git a/tests/ui/traits/new-solver/two-projection-param-candidates-are-ambiguous.stderr b/tests/ui/traits/new-solver/two-projection-param-candidates-are-ambiguous.stderr index fa5e780ee5e8b..83a0452b088d2 100644 --- a/tests/ui/traits/new-solver/two-projection-param-candidates-are-ambiguous.stderr +++ b/tests/ui/traits/new-solver/two-projection-param-candidates-are-ambiguous.stderr @@ -5,6 +5,7 @@ LL | needs_bar::(); | ^^^^^^^^^^^^^^ | = note: cannot satisfy `T: Bar` + = help: the trait `Bar` is implemented for `T` note: required by a bound in `needs_bar` --> $DIR/two-projection-param-candidates-are-ambiguous.rs:23:17 | From 42571c4847692edc050288372a107e51ab4a9ce4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 20 Jun 2023 21:27:43 +0000 Subject: [PATCH 2/3] yeet ImplSource::TraitAlias too --- compiler/rustc_middle/src/traits/mod.rs | 19 ------------------- .../src/traits/structural_impls.rs | 12 ------------ .../src/traits/project.rs | 4 +--- .../src/traits/select/confirmation.rs | 14 ++++++-------- compiler/rustc_ty_utils/src/instance.rs | 4 +--- 5 files changed, 8 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index f03802049707c..f2bda4adf48e7 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -660,9 +660,6 @@ pub enum ImplSource<'tcx, N> { /// ImplSource for trait upcasting coercion TraitUpcasting(ImplSourceTraitUpcastingData), - - /// ImplSource for a trait alias. - TraitAlias(ImplSourceTraitAliasData<'tcx, N>), } impl<'tcx, N> ImplSource<'tcx, N> { @@ -671,7 +668,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::UserDefined(i) => i.nested, ImplSource::Param(n, _) | ImplSource::Builtin(n) => n, ImplSource::Object(d) => d.nested, - ImplSource::TraitAlias(d) => d.nested, ImplSource::TraitUpcasting(d) => d.nested, } } @@ -681,7 +677,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::UserDefined(i) => &i.nested, ImplSource::Param(n, _) | ImplSource::Builtin(n) => &n, ImplSource::Object(d) => &d.nested, - ImplSource::TraitAlias(d) => &d.nested, ImplSource::TraitUpcasting(d) => &d.nested, } } @@ -691,7 +686,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::UserDefined(i) => &mut i.nested, ImplSource::Param(n, _) | ImplSource::Builtin(n) => n, ImplSource::Object(d) => &mut d.nested, - ImplSource::TraitAlias(d) => &mut d.nested, ImplSource::TraitUpcasting(d) => &mut d.nested, } } @@ -713,11 +707,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { vtable_base: o.vtable_base, nested: o.nested.into_iter().map(f).collect(), }), - ImplSource::TraitAlias(d) => ImplSource::TraitAlias(ImplSourceTraitAliasData { - alias_def_id: d.alias_def_id, - substs: d.substs, - nested: d.nested.into_iter().map(f).collect(), - }), ImplSource::TraitUpcasting(d) => { ImplSource::TraitUpcasting(ImplSourceTraitUpcastingData { vtable_vptr_slot: d.vtable_vptr_slot, @@ -773,14 +762,6 @@ pub struct ImplSourceObjectData { pub nested: Vec, } -#[derive(Clone, PartialEq, Eq, TyEncodable, TyDecodable, HashStable, Lift)] -#[derive(TypeFoldable, TypeVisitable)] -pub struct ImplSourceTraitAliasData<'tcx, N> { - pub alias_def_id: DefId, - pub substs: SubstsRef<'tcx>, - pub nested: Vec, -} - #[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)] pub enum ObjectSafetyViolation { /// `Self: Sized` declared on the trait. diff --git a/compiler/rustc_middle/src/traits/structural_impls.rs b/compiler/rustc_middle/src/traits/structural_impls.rs index ac02d6ed62f2e..1a89560188a91 100644 --- a/compiler/rustc_middle/src/traits/structural_impls.rs +++ b/compiler/rustc_middle/src/traits/structural_impls.rs @@ -17,8 +17,6 @@ impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSource<'tcx, N> { write!(f, "ImplSourceParamData({:?}, {:?})", n, ct) } - super::ImplSource::TraitAlias(ref d) => write!(f, "{:?}", d), - super::ImplSource::TraitUpcasting(ref d) => write!(f, "{:?}", d), } } @@ -53,13 +51,3 @@ impl fmt::Debug for traits::ImplSourceObjectData { ) } } - -impl<'tcx, N: fmt::Debug> fmt::Debug for traits::ImplSourceTraitAliasData<'tcx, N> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "ImplSourceTraitAliasData(alias_def_id={:?}, substs={:?}, nested={:?})", - self.alias_def_id, self.substs, self.nested - ) - } -} diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index eb4cb88fb0b1e..bdffb9235e5b2 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1721,7 +1721,6 @@ fn assemble_candidates_from_impls<'cx, 'tcx>( }; let eligible = match &impl_source { - super::ImplSource::TraitAlias(_) => true, super::ImplSource::UserDefined(impl_data) => { // We have to be careful when projecting out of an // impl because of specialization. If we are not in @@ -2013,8 +2012,7 @@ fn confirm_select_candidate<'cx, 'tcx>( } super::ImplSource::Object(_) | super::ImplSource::Param(..) - | super::ImplSource::TraitUpcasting(_) - | super::ImplSource::TraitAlias(..) => { + | super::ImplSource::TraitUpcasting(_) => { // we don't create Select candidates with this kind of resolution span_bug!( obligation.cause.span, diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 3c356978d5ca8..88941a7575534 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -27,10 +27,9 @@ use crate::traits::vtable::{ }; use crate::traits::{ BuiltinDerivedObligation, ImplDerivedObligation, ImplDerivedObligationCause, ImplSource, - ImplSourceObjectData, ImplSourceTraitAliasData, ImplSourceTraitUpcastingData, - ImplSourceUserDefinedData, Normalized, Obligation, ObligationCause, - OutputTypeParameterMismatch, PredicateObligation, Selection, SelectionError, - TraitNotObjectSafe, TraitObligation, Unimplemented, + ImplSourceObjectData, ImplSourceTraitUpcastingData, ImplSourceUserDefinedData, Normalized, + Obligation, ObligationCause, OutputTypeParameterMismatch, PredicateObligation, Selection, + SelectionError, TraitNotObjectSafe, TraitObligation, Unimplemented, }; use super::BuiltinImplConditions; @@ -105,7 +104,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { TraitAliasCandidate => { let data = self.confirm_trait_alias_candidate(obligation); - ImplSource::TraitAlias(data) + ImplSource::Builtin(data) } BuiltinObjectCandidate => { @@ -721,10 +720,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn confirm_trait_alias_candidate( &mut self, obligation: &TraitObligation<'tcx>, - ) -> ImplSourceTraitAliasData<'tcx, PredicateObligation<'tcx>> { + ) -> Vec> { debug!(?obligation, "confirm_trait_alias_candidate"); - let alias_def_id = obligation.predicate.def_id(); let predicate = self.infcx.instantiate_binder_with_placeholders(obligation.predicate); let trait_ref = predicate.trait_ref; let trait_def_id = trait_ref.def_id; @@ -741,7 +739,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { debug!(?trait_def_id, ?trait_obligations, "trait alias obligations"); - ImplSourceTraitAliasData { alias_def_id, substs, nested: trait_obligations } + trait_obligations } fn confirm_generator_candidate( diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 45b1075b6023a..2d75862014d0b 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -293,9 +293,7 @@ fn resolve_associated_item<'tcx>( None } } - traits::ImplSource::Param(..) - | traits::ImplSource::TraitAlias(..) - | traits::ImplSource::TraitUpcasting(_) => None, + traits::ImplSource::Param(..) | traits::ImplSource::TraitUpcasting(_) => None, }) } From 7d0a5c31f5b3b4b1dc47ce51898be8895c9f4637 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 20 Jun 2023 21:48:05 +0000 Subject: [PATCH 3/3] yeet upcast_trait_def_id from ImplSourceObjectData --- compiler/rustc_middle/src/traits/mod.rs | 4 ---- compiler/rustc_middle/src/traits/structural_impls.rs | 4 ++-- .../rustc_trait_selection/src/traits/select/confirmation.rs | 6 +----- compiler/rustc_trait_selection/src/traits/util.rs | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index f2bda4adf48e7..a333fbbb50671 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -703,7 +703,6 @@ impl<'tcx, N> ImplSource<'tcx, N> { ImplSource::Param(n, ct) => ImplSource::Param(n.into_iter().map(f).collect(), ct), ImplSource::Builtin(n) => ImplSource::Builtin(n.into_iter().map(f).collect()), ImplSource::Object(o) => ImplSource::Object(ImplSourceObjectData { - upcast_trait_def_id: o.upcast_trait_def_id, vtable_base: o.vtable_base, nested: o.nested.into_iter().map(f).collect(), }), @@ -750,9 +749,6 @@ pub struct ImplSourceTraitUpcastingData { #[derive(PartialEq, Eq, Clone, TyEncodable, TyDecodable, HashStable, Lift)] #[derive(TypeFoldable, TypeVisitable)] pub struct ImplSourceObjectData { - /// `Foo` upcast to the obligation trait. This will be some supertrait of `Foo`. - pub upcast_trait_def_id: DefId, - /// The vtable is formed by concatenating together the method lists of /// the base object trait and all supertraits, pointers to supertrait vtable will /// be provided when necessary; this is the start of `upcast_trait_ref`'s methods diff --git a/compiler/rustc_middle/src/traits/structural_impls.rs b/compiler/rustc_middle/src/traits/structural_impls.rs index 1a89560188a91..a703e3c956228 100644 --- a/compiler/rustc_middle/src/traits/structural_impls.rs +++ b/compiler/rustc_middle/src/traits/structural_impls.rs @@ -46,8 +46,8 @@ impl fmt::Debug for traits::ImplSourceObjectData { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "ImplSourceObjectData(upcast={:?}, vtable_base={}, nested={:?})", - self.upcast_trait_def_id, self.vtable_base, self.nested + "ImplSourceObjectData(vtable_base={}, nested={:?})", + self.vtable_base, self.nested ) } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 88941a7575534..bf485ee7b53ae 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -651,11 +651,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { (unnormalized_upcast_trait_ref, ty::Binder::dummy(object_trait_ref)), ); - Ok(ImplSourceObjectData { - upcast_trait_def_id: upcast_trait_ref.def_id(), - vtable_base, - nested, - }) + Ok(ImplSourceObjectData { vtable_base, nested }) } fn confirm_fn_pointer_candidate( diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index 906c357e8ca7e..4dc2f26a8ed17 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -248,7 +248,7 @@ pub fn get_vtable_index_of_object_method<'tcx, N>( ) -> Option { // Count number of methods preceding the one we are selecting and // add them to the total offset. - tcx.own_existential_vtable_entries(object.upcast_trait_def_id) + tcx.own_existential_vtable_entries(tcx.parent(method_def_id)) .iter() .copied() .position(|def_id| def_id == method_def_id)