From 2ea2ced2beec11b1c0e8d42e45a270684618367f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 07:50:39 +0000 Subject: [PATCH 01/15] Simplify derived obligation peeling --- .../src/traits/error_reporting/mod.rs | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 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 7a3579eb1cc85..8f4383b728d5e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -694,29 +694,19 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } ObligationCauseCode::ImplDerivedObligation( box ImplDerivedObligationCause { - derived: - DerivedObligationCause { - parent_code, - parent_trait_pred, - }, + derived, .. }, ) | ObligationCauseCode::BuiltinDerivedObligation( - DerivedObligationCause { - parent_code, - parent_trait_pred, - }, + derived, ) | ObligationCauseCode::DerivedObligation( - DerivedObligationCause { - parent_code, - parent_trait_pred, - }, + derived, ) => { peeled = true; - code = &parent_code; - trait_pred = *parent_trait_pred; + code = &derived.parent_code; + trait_pred = derived.parent_trait_pred; } _ => break, }; From 312d27d0a2aa0c47a00b6803c54a9281d2038c68 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 08:43:39 +0000 Subject: [PATCH 02/15] Remove some unnecessary clones --- compiler/rustc_middle/src/traits/mod.rs | 14 +++++--- .../rustc_trait_selection/src/traits/wf.rs | 36 +++++++++---------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 7 ++-- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index ffde1294ec655..e5d4dfe891eff 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -143,10 +143,6 @@ impl<'tcx> ObligationCause<'tcx> { ObligationCause { span, body_id: hir::CRATE_HIR_ID, code: None } } - pub fn make_mut_code(&mut self) -> &mut ObligationCauseCode<'tcx> { - Lrc::make_mut(self.code.get_or_insert_with(|| Lrc::new(MISC_OBLIGATION_CAUSE_CODE))) - } - pub fn span(&self, tcx: TyCtxt<'tcx>) -> Span { match *self.code() { ObligationCauseCode::CompareImplMethodObligation { .. } @@ -173,6 +169,16 @@ impl<'tcx> ObligationCause<'tcx> { None => Lrc::new(MISC_OBLIGATION_CAUSE_CODE), } } + + pub fn map_code( + &mut self, + f: impl FnOnce(Lrc>) -> Lrc>, + ) { + self.code = Some(f(match self.code.take() { + Some(code) => code, + None => Lrc::new(MISC_OBLIGATION_CAUSE_CODE), + })); + } } #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index de0ade64247df..1ee5b385f9a23 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -294,30 +294,28 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { let obligations = self.nominal_obligations(trait_ref.def_id, trait_ref.substs); debug!("compute_trait_ref obligations {:?}", obligations); - let cause = self.cause(traits::MiscObligation); let param_env = self.param_env; let depth = self.recursion_depth; let item = self.item; - let extend = |obligation: traits::PredicateObligation<'tcx>| { - let mut cause = cause.clone(); - if let Some(parent_trait_pred) = obligation.predicate.to_opt_poly_trait_pred() { - let derived_cause = traits::DerivedObligationCause { - parent_trait_pred, - parent_code: obligation.cause.clone_code(), - }; - *cause.make_mut_code() = - traits::ObligationCauseCode::DerivedObligation(derived_cause); + let extend = |traits::PredicateObligation { predicate, mut cause, .. }| { + if let Some(parent_trait_pred) = predicate.to_opt_poly_trait_pred() { + cause.map_code(|parent_code| { + { + traits::ObligationCauseCode::DerivedObligation( + traits::DerivedObligationCause { parent_trait_pred, parent_code }, + ) + } + .into() + }); + } else { + cause = traits::ObligationCause::misc(self.span, self.body_id); } extend_cause_with_original_assoc_item_obligation( - tcx, - trait_ref, - item, - &mut cause, - obligation.predicate, + tcx, trait_ref, item, &mut cause, predicate, ); - traits::Obligation::with_depth(cause, depth, param_env, obligation.predicate) + traits::Obligation::with_depth(cause, depth, param_env, predicate) }; if let Elaborate::All = elaborate { @@ -339,17 +337,17 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { }) .filter(|(_, arg)| !arg.has_escaping_bound_vars()) .map(|(i, arg)| { - let mut new_cause = cause.clone(); + let mut cause = traits::ObligationCause::misc(self.span, self.body_id); // The first subst is the self ty - use the correct span for it. if i == 0 { if let Some(hir::ItemKind::Impl(hir::Impl { self_ty, .. })) = item.map(|i| &i.kind) { - new_cause.span = self_ty.span; + cause.span = self_ty.span; } } traits::Obligation::with_depth( - new_cause, + cause, depth, param_env, ty::Binder::dummy(ty::PredicateKind::WellFormed(arg)).to_predicate(tcx), diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 277743e4a46c1..604228d57a30b 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -1668,13 +1668,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We make sure that only *one* argument matches the obligation failure // and we assign the obligation's span to its expression's. error.obligation.cause.span = args[ref_in].span; - let parent_code = error.obligation.cause.clone_code(); - *error.obligation.cause.make_mut_code() = + error.obligation.cause.map_code(|parent_code| { ObligationCauseCode::FunctionArgumentObligation { arg_hir_id: args[ref_in].hir_id, call_hir_id: expr.hir_id, parent_code, - }; + } + .into() + }); } else if error.obligation.cause.span == call_sp { // Make function calls point at the callee, not the whole thing. if let hir::ExprKind::Call(callee, _) = expr.kind { From 7e2e3d4ebe9ed4cac5dbf92221bd1d3731b60e57 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 08:50:32 +0000 Subject: [PATCH 03/15] Don't lose an obligation cause --- compiler/rustc_trait_selection/src/traits/wf.rs | 2 -- .../hr-associated-type-projection-1.stderr | 14 +++++++++++--- .../builtin-superkinds-self-type.stderr | 7 ++++++- src/test/ui/traits/assoc-type-in-superbad.stderr | 8 +++++++- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 1ee5b385f9a23..9752e8e73e79e 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -309,8 +309,6 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { } .into() }); - } else { - cause = traits::ObligationCause::misc(self.span, self.body_id); } extend_cause_with_original_assoc_item_obligation( tcx, trait_ref, item, &mut cause, predicate, diff --git a/src/test/ui/associated-types/hr-associated-type-projection-1.stderr b/src/test/ui/associated-types/hr-associated-type-projection-1.stderr index 9c29e969de8da..a65f84ae58ead 100644 --- a/src/test/ui/associated-types/hr-associated-type-projection-1.stderr +++ b/src/test/ui/associated-types/hr-associated-type-projection-1.stderr @@ -2,10 +2,18 @@ error[E0271]: type mismatch resolving `::Target == T` --> $DIR/hr-associated-type-projection-1.rs:13:33 | LL | impl UnsafeCopy<'_, T> for T { - | - this type parameter ^^^^^^^^^^^^^^^^^ expected associated type, found type parameter `T` + | - this type parameter ^^^^^^^^^^^^^^^^^ expected type parameter `T`, found associated type | - = note: expected associated type `::Target` - found type parameter `T` + = note: expected type parameter `T` + found associated type `::Target` +note: required by a bound in `UnsafeCopy` + --> $DIR/hr-associated-type-projection-1.rs:3:64 + | +LL | trait UnsafeCopy<'a, T: Copy> + | ---------- required by a bound in this +LL | where +LL | for<'b> >::Item: std::ops::Deref, + | ^^^^^^^^^^ required by this bound in `UnsafeCopy` help: consider further restricting this bound | LL | impl> UnsafeCopy<'_, T> for T { diff --git a/src/test/ui/builtin-superkinds/builtin-superkinds-self-type.stderr b/src/test/ui/builtin-superkinds/builtin-superkinds-self-type.stderr index b1e59e9d5de36..e2b177b951cc9 100644 --- a/src/test/ui/builtin-superkinds/builtin-superkinds-self-type.stderr +++ b/src/test/ui/builtin-superkinds/builtin-superkinds-self-type.stderr @@ -2,8 +2,13 @@ error[E0310]: the parameter type `T` may not live long enough --> $DIR/builtin-superkinds-self-type.rs:10:16 | LL | impl Foo for T { } - | ^^^ ...so that the type `T` will meet its required lifetime bounds + | ^^^ ...so that the type `T` will meet its required lifetime bounds... | +note: ...that is required by this bound + --> $DIR/builtin-superkinds-self-type.rs:6:24 + | +LL | trait Foo : Sized+Sync+'static { + | ^^^^^^^ help: consider adding an explicit lifetime bound... | LL | impl Foo for T { } diff --git a/src/test/ui/traits/assoc-type-in-superbad.stderr b/src/test/ui/traits/assoc-type-in-superbad.stderr index cbdb6b96f468f..f36947914179c 100644 --- a/src/test/ui/traits/assoc-type-in-superbad.stderr +++ b/src/test/ui/traits/assoc-type-in-superbad.stderr @@ -2,7 +2,13 @@ error[E0271]: type mismatch resolving ` as Iterator>::It --> $DIR/assoc-type-in-superbad.rs:12:16 | LL | type Key = u32; - | ^^^ expected `i32`, found `u32` + | ^^^ expected `u32`, found `i32` + | +note: required by a bound in `Foo` + --> $DIR/assoc-type-in-superbad.rs:7:25 + | +LL | pub trait Foo: Iterator::Key> { + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Foo` error: aborting due to previous error From 704bbe5210613c40fb997adba2375e54ea29b205 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 09:26:09 +0000 Subject: [PATCH 04/15] Move an extension trait method onto the type directly and reuse it --- compiler/rustc_infer/src/traits/mod.rs | 9 ++++- compiler/rustc_middle/src/traits/mod.rs | 24 +++++++++++++ .../src/traits/select/confirmation.rs | 1 - .../src/traits/select/mod.rs | 36 ------------------- .../rustc_trait_selection/src/traits/wf.rs | 12 +++---- 5 files changed, 36 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_infer/src/traits/mod.rs b/compiler/rustc_infer/src/traits/mod.rs index 85bb727a6c804..f34a5395cfe7c 100644 --- a/compiler/rustc_infer/src/traits/mod.rs +++ b/compiler/rustc_infer/src/traits/mod.rs @@ -69,7 +69,7 @@ impl<'tcx> PredicateObligation<'tcx> { } } -impl TraitObligation<'_> { +impl<'tcx> TraitObligation<'tcx> { /// Returns `true` if the trait predicate is considered `const` in its ParamEnv. pub fn is_const(&self) -> bool { match (self.predicate.skip_binder().constness, self.param_env.constness()) { @@ -77,6 +77,13 @@ impl TraitObligation<'_> { _ => false, } } + + pub fn derived_cause( + &self, + variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, + ) -> ObligationCause<'tcx> { + self.cause.clone().derived_cause(self.predicate, variant) + } } // `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index e5d4dfe891eff..550f5a086e944 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -179,6 +179,30 @@ impl<'tcx> ObligationCause<'tcx> { None => Lrc::new(MISC_OBLIGATION_CAUSE_CODE), })); } + + pub fn derived_cause( + mut self, + parent_trait_pred: ty::PolyTraitPredicate<'tcx>, + variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, + ) -> ObligationCause<'tcx> { + /*! + * Creates a cause for obligations that are derived from + * `obligation` by a recursive search (e.g., for a builtin + * bound, or eventually a `auto trait Foo`). If `obligation` + * is itself a derived obligation, this is just a clone, but + * otherwise we create a "derived obligation" cause so as to + * keep track of the original root obligation for error + * reporting. + */ + + // NOTE(flaper87): As of now, it keeps track of the whole error + // chain. Ideally, we should have a way to configure this either + // by using -Z verbose or just a CLI argument. + self.map_code(|parent_code| { + variant(DerivedObligationCause { parent_trait_pred, parent_code }).into() + }); + self + } } #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 771a3072af5d9..613ac8a760eff 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -17,7 +17,6 @@ use rustc_middle::ty::{ToPolyTraitRef, ToPredicate}; use rustc_span::def_id::DefId; use crate::traits::project::{normalize_with_depth, normalize_with_depth_to}; -use crate::traits::select::TraitObligationExt; use crate::traits::util::{self, closure_trait_ref_and_return_type, predicate_for_trait_def}; use crate::traits::{ BuiltinDerivedObligation, DerivedObligationCause, ImplDerivedObligation, diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 6584d33032a46..0790e5d56d5f1 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2342,42 +2342,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } -trait TraitObligationExt<'tcx> { - fn derived_cause( - &self, - variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, - ) -> ObligationCause<'tcx>; -} - -impl<'tcx> TraitObligationExt<'tcx> for TraitObligation<'tcx> { - fn derived_cause( - &self, - variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, - ) -> ObligationCause<'tcx> { - /*! - * Creates a cause for obligations that are derived from - * `obligation` by a recursive search (e.g., for a builtin - * bound, or eventually a `auto trait Foo`). If `obligation` - * is itself a derived obligation, this is just a clone, but - * otherwise we create a "derived obligation" cause so as to - * keep track of the original root obligation for error - * reporting. - */ - - let obligation = self; - - // NOTE(flaper87): As of now, it keeps track of the whole error - // chain. Ideally, we should have a way to configure this either - // by using -Z verbose or just a CLI argument. - let derived_cause = DerivedObligationCause { - parent_trait_pred: obligation.predicate, - parent_code: obligation.cause.clone_code(), - }; - let derived_code = variant(derived_cause); - ObligationCause::new(obligation.cause.span, obligation.cause.body_id, derived_code) - } -} - impl<'o, 'tcx> TraitObligationStack<'o, 'tcx> { fn list(&'o self) -> TraitObligationStackList<'o, 'tcx> { TraitObligationStackList::with(self) diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 9752e8e73e79e..18afc6dbcd805 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -301,14 +301,10 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { let extend = |traits::PredicateObligation { predicate, mut cause, .. }| { if let Some(parent_trait_pred) = predicate.to_opt_poly_trait_pred() { - cause.map_code(|parent_code| { - { - traits::ObligationCauseCode::DerivedObligation( - traits::DerivedObligationCause { parent_trait_pred, parent_code }, - ) - } - .into() - }); + cause = cause.derived_cause( + parent_trait_pred, + traits::ObligationCauseCode::DerivedObligation, + ); } extend_cause_with_original_assoc_item_obligation( tcx, trait_ref, item, &mut cause, predicate, From 063795ce4aa58c91cb1f927ca95188c9b0f35608 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 10:26:43 +0000 Subject: [PATCH 05/15] Remove another use of clone_code --- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 604228d57a30b..43fb703456a46 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -12,7 +12,6 @@ use crate::check::{ use crate::structured_errors::StructuredDiagnostic; use rustc_ast as ast; -use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, Diagnostic, DiagnosticId, MultiSpan}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; @@ -1601,24 +1600,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Peel derived obligation, because it's the type that originally // started this inference chain that matters, not the one we wound // up with at the end. - fn unpeel_to_top( - mut code: Lrc>, - ) -> Lrc> { - let mut result_code = code.clone(); + fn unpeel_to_top<'a, 'tcx>( + mut code: &'a ObligationCauseCode<'tcx>, + ) -> &'a ObligationCauseCode<'tcx> { + let mut result_code = code; loop { - let parent = match &*code { - ObligationCauseCode::ImplDerivedObligation(c) => { - c.derived.parent_code.clone() - } + let parent = match code { + ObligationCauseCode::ImplDerivedObligation(c) => &c.derived.parent_code, ObligationCauseCode::BuiltinDerivedObligation(c) - | ObligationCauseCode::DerivedObligation(c) => c.parent_code.clone(), - _ => break, + | ObligationCauseCode::DerivedObligation(c) => &c.parent_code, + _ => break result_code, }; - result_code = std::mem::replace(&mut code, parent); + (result_code, code) = (code, parent); } - result_code } - let self_: ty::subst::GenericArg<'_> = match &*unpeel_to_top(error.obligation.cause.clone_code()) { + let self_: ty::subst::GenericArg<'_> = match unpeel_to_top(error.obligation.cause.code()) { ObligationCauseCode::BuiltinDerivedObligation(code) | ObligationCauseCode::DerivedObligation(code) => { code.parent_trait_pred.self_ty().skip_binder().into() From dc21fcb2fccbb10f0ffd7c6cc07e323905555a23 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 10:32:35 +0000 Subject: [PATCH 06/15] Remove another use of clone_code --- compiler/rustc_middle/src/traits/mod.rs | 2 +- .../src/traits/select/mod.rs | 22 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 550f5a086e944..ed8a399480145 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -183,7 +183,7 @@ impl<'tcx> ObligationCause<'tcx> { pub fn derived_cause( mut self, parent_trait_pred: ty::PolyTraitPredicate<'tcx>, - variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, + variant: impl FnOnce(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, ) -> ObligationCause<'tcx> { /*! * Creates a cause for obligations that are derived from diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 0790e5d56d5f1..0683f61b47101 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -14,9 +14,9 @@ use super::util; use super::util::{closure_trait_ref_and_return_type, predicate_for_trait_def}; use super::wf; use super::{ - DerivedObligationCause, ErrorReporting, ImplDerivedObligation, ImplDerivedObligationCause, - Normalized, Obligation, ObligationCause, ObligationCauseCode, Overflow, PredicateObligation, - Selection, SelectionError, SelectionResult, TraitObligation, TraitQueryMode, + ErrorReporting, ImplDerivedObligation, ImplDerivedObligationCause, Normalized, Obligation, + ObligationCause, ObligationCauseCode, Overflow, PredicateObligation, Selection, SelectionError, + SelectionResult, TraitObligation, TraitQueryMode, }; use crate::infer::{InferCtxt, InferOk, TypeFreshener}; @@ -2316,17 +2316,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { debug!(?predicates); assert_eq!(predicates.parent, None); let mut obligations = Vec::with_capacity(predicates.predicates.len()); - let parent_code = cause.clone_code(); for (predicate, span) in predicates.predicates { let span = *span; - let derived = - DerivedObligationCause { parent_trait_pred, parent_code: parent_code.clone() }; - let code = ImplDerivedObligation(Box::new(ImplDerivedObligationCause { - derived, - impl_def_id: def_id, - span, - })); - let cause = ObligationCause::new(cause.span, cause.body_id, code); + let cause = cause.clone().derived_cause(parent_trait_pred, |derived| { + ImplDerivedObligation(Box::new(ImplDerivedObligationCause { + derived, + impl_def_id: def_id, + span, + })) + }); let predicate = normalize_with_depth_to( self, param_env, From 05a62c55270020252a0e2aab5a57ad6c2ce77aef Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 10:42:29 +0000 Subject: [PATCH 07/15] Remove `clone_code` method --- compiler/rustc_infer/src/traits/mod.rs | 2 +- compiler/rustc_middle/src/traits/mod.rs | 7 ---- .../src/traits/select/confirmation.rs | 37 +++++++------------ 3 files changed, 15 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_infer/src/traits/mod.rs b/compiler/rustc_infer/src/traits/mod.rs index f34a5395cfe7c..4df4de21a0f07 100644 --- a/compiler/rustc_infer/src/traits/mod.rs +++ b/compiler/rustc_infer/src/traits/mod.rs @@ -80,7 +80,7 @@ impl<'tcx> TraitObligation<'tcx> { pub fn derived_cause( &self, - variant: fn(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, + variant: impl FnOnce(DerivedObligationCause<'tcx>) -> ObligationCauseCode<'tcx>, ) -> ObligationCause<'tcx> { self.cause.clone().derived_cause(self.predicate, variant) } diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index ed8a399480145..31744671d3a16 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -163,13 +163,6 @@ impl<'tcx> ObligationCause<'tcx> { self.code.as_deref().unwrap_or(&MISC_OBLIGATION_CAUSE_CODE) } - pub fn clone_code(&self) -> Lrc> { - match &self.code { - Some(code) => code.clone(), - None => Lrc::new(MISC_OBLIGATION_CAUSE_CODE), - } - } - pub fn map_code( &mut self, f: impl FnOnce(Lrc>) -> Lrc>, diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 613ac8a760eff..0650e847d9ea3 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -19,14 +19,13 @@ use rustc_span::def_id::DefId; use crate::traits::project::{normalize_with_depth, normalize_with_depth_to}; use crate::traits::util::{self, closure_trait_ref_and_return_type, predicate_for_trait_def}; use crate::traits::{ - BuiltinDerivedObligation, DerivedObligationCause, ImplDerivedObligation, - ImplDerivedObligationCause, ImplSource, ImplSourceAutoImplData, ImplSourceBuiltinData, - ImplSourceClosureData, ImplSourceConstDestructData, ImplSourceDiscriminantKindData, - ImplSourceFnPointerData, ImplSourceGeneratorData, ImplSourceObjectData, ImplSourcePointeeData, - ImplSourceTraitAliasData, ImplSourceTraitUpcastingData, ImplSourceUserDefinedData, Normalized, - ObjectCastObligation, Obligation, ObligationCause, OutputTypeParameterMismatch, - PredicateObligation, Selection, SelectionError, TraitNotObjectSafe, TraitObligation, - Unimplemented, VtblSegment, + BuiltinDerivedObligation, ImplDerivedObligation, ImplDerivedObligationCause, ImplSource, + ImplSourceAutoImplData, ImplSourceBuiltinData, ImplSourceClosureData, + ImplSourceConstDestructData, ImplSourceDiscriminantKindData, ImplSourceFnPointerData, + ImplSourceGeneratorData, ImplSourceObjectData, ImplSourcePointeeData, ImplSourceTraitAliasData, + ImplSourceTraitUpcastingData, ImplSourceUserDefinedData, Normalized, ObjectCastObligation, + Obligation, ObligationCause, OutputTypeParameterMismatch, PredicateObligation, Selection, + SelectionError, TraitNotObjectSafe, TraitObligation, Unimplemented, VtblSegment, }; use super::BuiltinImplConditions; @@ -1125,21 +1124,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let substs = self.rematch_impl(impl_def_id, &new_obligation); debug!(?substs, "impl substs"); - let derived = DerivedObligationCause { - parent_trait_pred: obligation.predicate, - parent_code: obligation.cause.clone_code(), - }; - let derived_code = ImplDerivedObligation(Box::new(ImplDerivedObligationCause { - derived, - impl_def_id, - span: obligation.cause.span, - })); - - let cause = ObligationCause::new( - obligation.cause.span, - obligation.cause.body_id, - derived_code, - ); + let cause = obligation.derived_cause(|derived| { + ImplDerivedObligation(Box::new(ImplDerivedObligationCause { + derived, + impl_def_id, + span: obligation.cause.span, + })) + }); ensure_sufficient_stack(|| { self.vtable_impl( impl_def_id, From 5b5b54958022911b5ba9afc85b7f58d69e0842f8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 11:03:52 +0000 Subject: [PATCH 08/15] Add a helper function for a common piece of code --- compiler/rustc_middle/src/traits/mod.rs | 28 +++++++------ .../src/traits/error_reporting/mod.rs | 41 +++++-------------- .../src/traits/error_reporting/suggestions.rs | 31 ++++---------- 3 files changed, 32 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 31744671d3a16..d28605fc26f38 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -427,25 +427,27 @@ pub struct ImplDerivedObligationCause<'tcx> { pub span: Span, } -impl ObligationCauseCode<'_> { +impl<'tcx> ObligationCauseCode<'tcx> { // Return the base obligation, ignoring derived obligations. pub fn peel_derives(&self) -> &Self { let mut base_cause = self; - loop { - match base_cause { - BuiltinDerivedObligation(DerivedObligationCause { parent_code, .. }) - | DerivedObligation(DerivedObligationCause { parent_code, .. }) - | FunctionArgumentObligation { parent_code, .. } => { - base_cause = &parent_code; - } - ImplDerivedObligation(obligation_cause) => { - base_cause = &*obligation_cause.derived.parent_code; - } - _ => break, - } + while let Some((parent_code, _)) = base_cause.parent() { + base_cause = parent_code; } base_cause } + + pub fn parent(&self) -> Option<(&Self, Option>)> { + match self { + FunctionArgumentObligation { parent_code, .. } => Some((parent_code, None)), + BuiltinDerivedObligation(derived) + | DerivedObligation(derived) + | ImplDerivedObligation(box ImplDerivedObligationCause { derived, .. }) => { + Some((&derived.parent_code, Some(derived.parent_trait_pred))) + } + _ => None, + } + } } // `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. 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 8f4383b728d5e..6082d7529c32e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2,11 +2,10 @@ pub mod on_unimplemented; pub mod suggestions; use super::{ - DerivedObligationCause, EvaluationResult, FulfillmentContext, FulfillmentError, - FulfillmentErrorCode, ImplDerivedObligationCause, MismatchedProjectionTypes, Obligation, - ObligationCause, ObligationCauseCode, OnUnimplementedDirective, OnUnimplementedNote, - OutputTypeParameterMismatch, Overflow, PredicateObligation, SelectionContext, SelectionError, - TraitNotObjectSafe, + EvaluationResult, FulfillmentContext, FulfillmentError, FulfillmentErrorCode, + MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode, + OnUnimplementedDirective, OnUnimplementedNote, OutputTypeParameterMismatch, Overflow, + PredicateObligation, SelectionContext, SelectionError, TraitNotObjectSafe, }; use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode}; @@ -684,32 +683,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let mut code = obligation.cause.code(); let mut trait_pred = trait_predicate; let mut peeled = false; - loop { - match &*code { - ObligationCauseCode::FunctionArgumentObligation { - parent_code, - .. - } => { - code = &parent_code; - } - ObligationCauseCode::ImplDerivedObligation( - box ImplDerivedObligationCause { - derived, - .. - }, - ) - | ObligationCauseCode::BuiltinDerivedObligation( - derived, - ) - | ObligationCauseCode::DerivedObligation( - derived, - ) => { - peeled = true; - code = &derived.parent_code; - trait_pred = derived.parent_trait_pred; - } - _ => break, - }; + while let Some((parent_code, parent_trait_pred)) = code.parent() { + code = parent_code; + if let Some(parent_trait_pred) = parent_trait_pred { + trait_pred = parent_trait_pred; + peeled = true; + } } let def_id = trait_pred.def_id(); // Mention *all* the `impl`s for the *top most* obligation, the diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index d20ba99ebc9b5..e4060d1ee5bb8 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1,6 +1,6 @@ use super::{ - DerivedObligationCause, EvaluationResult, ImplDerivedObligationCause, Obligation, - ObligationCause, ObligationCauseCode, PredicateObligation, SelectionContext, + EvaluationResult, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation, + SelectionContext, }; use crate::autoderef::Autoderef; @@ -623,28 +623,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let span = obligation.cause.span; let mut real_trait_pred = trait_pred; let mut code = obligation.cause.code(); - loop { - match &code { - ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => { - code = &parent_code; - } - ObligationCauseCode::ImplDerivedObligation(box ImplDerivedObligationCause { - derived: DerivedObligationCause { parent_code, parent_trait_pred }, - .. - }) - | ObligationCauseCode::BuiltinDerivedObligation(DerivedObligationCause { - parent_code, - parent_trait_pred, - }) - | ObligationCauseCode::DerivedObligation(DerivedObligationCause { - parent_code, - parent_trait_pred, - }) => { - code = &parent_code; - real_trait_pred = *parent_trait_pred; - } - _ => break, - }; + while let Some((parent_code, parent_trait_pred)) = code.parent() { + code = parent_code; + if let Some(parent_trait_pred) = parent_trait_pred { + real_trait_pred = parent_trait_pred; + } let Some(real_ty) = real_trait_pred.self_ty().no_bound_vars() else { continue; }; From 9ba6ddb929563e221f0b4cd32949218148b9a1c1 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 11:10:27 +0000 Subject: [PATCH 09/15] Make the derived obligation cause parent private --- compiler/rustc_middle/src/traits/mod.rs | 10 +++++++++- .../src/traits/error_reporting/mod.rs | 2 +- .../src/traits/error_reporting/suggestions.rs | 19 +++++++++---------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 4 ++-- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index d28605fc26f38..be2d8def95437 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -497,7 +497,15 @@ pub struct DerivedObligationCause<'tcx> { pub parent_trait_pred: ty::PolyTraitPredicate<'tcx>, /// The parent trait had this cause. - pub parent_code: Lrc>, + parent_code: Lrc>, +} + +impl<'tcx> DerivedObligationCause<'tcx> { + /// Get a reference to the derived obligation cause's parent code. + #[must_use] + pub fn parent_code(&self) -> &ObligationCauseCode<'tcx> { + self.parent_code.as_ref() + } } #[derive(Clone, Debug, TypeFoldable, Lift)] 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 6082d7529c32e..81e62f6da06e9 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1868,7 +1868,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { match code { ObligationCauseCode::BuiltinDerivedObligation(data) => { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); - match self.get_parent_trait_ref(&data.parent_code) { + match self.get_parent_trait_ref(data.parent_code()) { Some(t) => Some(t), None => { let ty = parent_trait_ref.skip_binder().self_ty(); diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index e4060d1ee5bb8..1522eedf7fe77 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1683,7 +1683,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => {} } - next_code = Some(cause.derived.parent_code.as_ref()); + next_code = Some(cause.derived.parent_code()); } ObligationCauseCode::DerivedObligation(derived_obligation) | ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) => { @@ -1715,7 +1715,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => {} } - next_code = Some(derived_obligation.parent_code.as_ref()); + next_code = Some(derived_obligation.parent_code()); } _ => break, } @@ -2365,8 +2365,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let is_upvar_tys_infer_tuple = if !matches!(ty.kind(), ty::Tuple(..)) { false } else { - if let ObligationCauseCode::BuiltinDerivedObligation(ref data) = - *data.parent_code + if let ObligationCauseCode::BuiltinDerivedObligation(data) = data.parent_code() { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); @@ -2393,14 +2392,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { obligated_types.push(ty); let parent_predicate = parent_trait_ref.to_predicate(tcx); - if !self.is_recursive_obligation(obligated_types, &data.parent_code) { + if !self.is_recursive_obligation(obligated_types, data.parent_code()) { // #74711: avoid a stack overflow ensure_sufficient_stack(|| { self.note_obligation_cause_code( err, &parent_predicate, param_env, - &data.parent_code, + data.parent_code(), obligated_types, seen_requirements, ) @@ -2462,7 +2461,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // We don't want to point at the ADT saying "required because it appears within // the type `X`", like we would otherwise do in test `supertrait-auto-trait.rs`. while let ObligationCauseCode::BuiltinDerivedObligation(derived) = - &*data.parent_code + data.parent_code() { let child_trait_ref = self.resolve_vars_if_possible(derived.parent_trait_pred); @@ -2475,7 +2474,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { parent_trait_pred = child_trait_ref; } } - while let ObligationCauseCode::ImplDerivedObligation(child) = &*data.parent_code { + while let ObligationCauseCode::ImplDerivedObligation(child) = data.parent_code() { // Skip redundant recursive obligation notes. See `ui/issue-20413.rs`. let child_trait_pred = self.resolve_vars_if_possible(child.derived.parent_trait_pred); @@ -2506,7 +2505,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err, &parent_predicate, param_env, - &data.parent_code, + data.parent_code(), obligated_types, seen_requirements, ) @@ -2521,7 +2520,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err, &parent_predicate, param_env, - &data.parent_code, + data.parent_code(), obligated_types, seen_requirements, ) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 43fb703456a46..300b87aa46575 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -1606,9 +1606,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut result_code = code; loop { let parent = match code { - ObligationCauseCode::ImplDerivedObligation(c) => &c.derived.parent_code, + ObligationCauseCode::ImplDerivedObligation(c) => c.derived.parent_code(), ObligationCauseCode::BuiltinDerivedObligation(c) - | ObligationCauseCode::DerivedObligation(c) => &c.parent_code, + | ObligationCauseCode::DerivedObligation(c) => c.parent_code(), _ => break result_code, }; (result_code, code) = (code, parent); From 1b51e1ad2071a5cd31013a75df653b64dc6ca9c5 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 11:14:07 +0000 Subject: [PATCH 10/15] Don't allocate misc obligation parents of derived obligations --- compiler/rustc_middle/src/traits/mod.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index be2d8def95437..04e3daf304536 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -191,9 +191,10 @@ impl<'tcx> ObligationCause<'tcx> { // NOTE(flaper87): As of now, it keeps track of the whole error // chain. Ideally, we should have a way to configure this either // by using -Z verbose or just a CLI argument. - self.map_code(|parent_code| { - variant(DerivedObligationCause { parent_trait_pred, parent_code }).into() - }); + self.code = Some( + variant(DerivedObligationCause { parent_trait_pred, parent_code: self.code.take() }) + .into(), + ); self } } @@ -443,7 +444,7 @@ impl<'tcx> ObligationCauseCode<'tcx> { BuiltinDerivedObligation(derived) | DerivedObligation(derived) | ImplDerivedObligation(box ImplDerivedObligationCause { derived, .. }) => { - Some((&derived.parent_code, Some(derived.parent_trait_pred))) + Some((derived.parent_code(), Some(derived.parent_trait_pred))) } _ => None, } @@ -497,14 +498,14 @@ pub struct DerivedObligationCause<'tcx> { pub parent_trait_pred: ty::PolyTraitPredicate<'tcx>, /// The parent trait had this cause. - parent_code: Lrc>, + parent_code: Option>>, } impl<'tcx> DerivedObligationCause<'tcx> { /// Get a reference to the derived obligation cause's parent code. #[must_use] pub fn parent_code(&self) -> &ObligationCauseCode<'tcx> { - self.parent_code.as_ref() + self.parent_code.as_deref().unwrap_or(&MISC_OBLIGATION_CAUSE_CODE) } } From 213c17486e26aa88b6088f0f015bb86cde6fdf8c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 11:26:53 +0000 Subject: [PATCH 11/15] Make `FunctionArgumentObligation` also use the "no allocation for misc" trick --- compiler/rustc_middle/src/traits/mod.rs | 22 ++++++++++++++----- .../src/traits/error_reporting/suggestions.rs | 2 +- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 04e3daf304536..664eef7ca5691 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -165,12 +165,9 @@ impl<'tcx> ObligationCause<'tcx> { pub fn map_code( &mut self, - f: impl FnOnce(Lrc>) -> Lrc>, + f: impl FnOnce(InternedObligationCauseCode<'tcx>) -> Lrc>, ) { - self.code = Some(f(match self.code.take() { - Some(code) => code, - None => Lrc::new(MISC_OBLIGATION_CAUSE_CODE), - })); + self.code = Some(f(InternedObligationCauseCode { code: self.code.take() })); } pub fn derived_cause( @@ -206,6 +203,19 @@ pub struct UnifyReceiverContext<'tcx> { pub substs: SubstsRef<'tcx>, } +#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] +pub struct InternedObligationCauseCode<'tcx> { + code: Option>>, +} + +impl<'tcx> std::ops::Deref for InternedObligationCauseCode<'tcx> { + type Target = ObligationCauseCode<'tcx>; + + fn deref(&self) -> &Self::Target { + self.code.as_deref().unwrap_or(&MISC_OBLIGATION_CAUSE_CODE) + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] pub enum ObligationCauseCode<'tcx> { /// Not well classified or should be obvious from the span. @@ -293,7 +303,7 @@ pub enum ObligationCauseCode<'tcx> { /// The node of the function call. call_hir_id: hir::HirId, /// The obligation introduced by this argument. - parent_code: Lrc>, + parent_code: InternedObligationCauseCode<'tcx>, }, /// Error derived when matching traits/impls; see ObligationCause for more details diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 1522eedf7fe77..ee3e9544b4d60 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1652,7 +1652,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code); match code { ObligationCauseCode::FunctionArgumentObligation { parent_code, .. } => { - next_code = Some(parent_code.as_ref()); + next_code = Some(parent_code); } ObligationCauseCode::ImplDerivedObligation(cause) => { let ty = cause.derived.parent_trait_pred.skip_binder().self_ty(); From 824e9e47f7ae70970a7f174976e43935fdfee98d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 10 May 2022 12:01:56 +0000 Subject: [PATCH 12/15] Use InternedObligationCauseCode everywhere --- compiler/rustc_middle/src/traits/mod.rs | 48 ++++++++----------- .../src/traits/error_reporting/mod.rs | 2 +- .../src/traits/error_reporting/suggestions.rs | 20 ++++---- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 5 +- 4 files changed, 33 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 664eef7ca5691..4b67fc84d2ed8 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -97,9 +97,7 @@ pub struct ObligationCause<'tcx> { /// information. pub body_id: hir::HirId, - /// `None` for `MISC_OBLIGATION_CAUSE_CODE` (a common case, occurs ~60% of - /// the time). `Some` otherwise. - code: Option>>, + code: InternedObligationCauseCode<'tcx>, } // This custom hash function speeds up hashing for `Obligation` deduplication @@ -123,11 +121,7 @@ impl<'tcx> ObligationCause<'tcx> { body_id: hir::HirId, code: ObligationCauseCode<'tcx>, ) -> ObligationCause<'tcx> { - ObligationCause { - span, - body_id, - code: if code == MISC_OBLIGATION_CAUSE_CODE { None } else { Some(Lrc::new(code)) }, - } + ObligationCause { span, body_id, code: code.into() } } pub fn misc(span: Span, body_id: hir::HirId) -> ObligationCause<'tcx> { @@ -136,11 +130,11 @@ impl<'tcx> ObligationCause<'tcx> { #[inline(always)] pub fn dummy() -> ObligationCause<'tcx> { - ObligationCause { span: DUMMY_SP, body_id: hir::CRATE_HIR_ID, code: None } + ObligationCause::dummy_with_span(DUMMY_SP) } pub fn dummy_with_span(span: Span) -> ObligationCause<'tcx> { - ObligationCause { span, body_id: hir::CRATE_HIR_ID, code: None } + ObligationCause { span, body_id: hir::CRATE_HIR_ID, code: Default::default() } } pub fn span(&self, tcx: TyCtxt<'tcx>) -> Span { @@ -160,14 +154,14 @@ impl<'tcx> ObligationCause<'tcx> { #[inline] pub fn code(&self) -> &ObligationCauseCode<'tcx> { - self.code.as_deref().unwrap_or(&MISC_OBLIGATION_CAUSE_CODE) + &self.code } pub fn map_code( &mut self, - f: impl FnOnce(InternedObligationCauseCode<'tcx>) -> Lrc>, + f: impl FnOnce(InternedObligationCauseCode<'tcx>) -> ObligationCauseCode<'tcx>, ) { - self.code = Some(f(InternedObligationCauseCode { code: self.code.take() })); + self.code = f(std::mem::take(&mut self.code)).into(); } pub fn derived_cause( @@ -188,10 +182,8 @@ impl<'tcx> ObligationCause<'tcx> { // NOTE(flaper87): As of now, it keeps track of the whole error // chain. Ideally, we should have a way to configure this either // by using -Z verbose or just a CLI argument. - self.code = Some( - variant(DerivedObligationCause { parent_trait_pred, parent_code: self.code.take() }) - .into(), - ); + self.code = + variant(DerivedObligationCause { parent_trait_pred, parent_code: self.code }).into(); self } } @@ -203,11 +195,19 @@ pub struct UnifyReceiverContext<'tcx> { pub substs: SubstsRef<'tcx>, } -#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Lift, Default)] pub struct InternedObligationCauseCode<'tcx> { + /// `None` for `MISC_OBLIGATION_CAUSE_CODE` (a common case, occurs ~60% of + /// the time). `Some` otherwise. code: Option>>, } +impl<'tcx> From> for InternedObligationCauseCode<'tcx> { + fn from(code: ObligationCauseCode<'tcx>) -> Self { + Self { code: if code == MISC_OBLIGATION_CAUSE_CODE { None } else { Some(Lrc::new(code)) } } + } +} + impl<'tcx> std::ops::Deref for InternedObligationCauseCode<'tcx> { type Target = ObligationCauseCode<'tcx>; @@ -454,7 +454,7 @@ impl<'tcx> ObligationCauseCode<'tcx> { BuiltinDerivedObligation(derived) | DerivedObligation(derived) | ImplDerivedObligation(box ImplDerivedObligationCause { derived, .. }) => { - Some((derived.parent_code(), Some(derived.parent_trait_pred))) + Some((&derived.parent_code, Some(derived.parent_trait_pred))) } _ => None, } @@ -508,15 +508,7 @@ pub struct DerivedObligationCause<'tcx> { pub parent_trait_pred: ty::PolyTraitPredicate<'tcx>, /// The parent trait had this cause. - parent_code: Option>>, -} - -impl<'tcx> DerivedObligationCause<'tcx> { - /// Get a reference to the derived obligation cause's parent code. - #[must_use] - pub fn parent_code(&self) -> &ObligationCauseCode<'tcx> { - self.parent_code.as_deref().unwrap_or(&MISC_OBLIGATION_CAUSE_CODE) - } + pub parent_code: InternedObligationCauseCode<'tcx>, } #[derive(Clone, Debug, TypeFoldable, Lift)] 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 81e62f6da06e9..6082d7529c32e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1868,7 +1868,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> { match code { ObligationCauseCode::BuiltinDerivedObligation(data) => { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); - match self.get_parent_trait_ref(data.parent_code()) { + match self.get_parent_trait_ref(&data.parent_code) { Some(t) => Some(t), None => { let ty = parent_trait_ref.skip_binder().self_ty(); diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index ee3e9544b4d60..833e232e63665 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -1683,7 +1683,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => {} } - next_code = Some(cause.derived.parent_code()); + next_code = Some(&cause.derived.parent_code); } ObligationCauseCode::DerivedObligation(derived_obligation) | ObligationCauseCode::BuiltinDerivedObligation(derived_obligation) => { @@ -1715,7 +1715,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => {} } - next_code = Some(derived_obligation.parent_code()); + next_code = Some(&derived_obligation.parent_code); } _ => break, } @@ -2365,7 +2365,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { let is_upvar_tys_infer_tuple = if !matches!(ty.kind(), ty::Tuple(..)) { false } else { - if let ObligationCauseCode::BuiltinDerivedObligation(data) = data.parent_code() + if let ObligationCauseCode::BuiltinDerivedObligation(data) = &*data.parent_code { let parent_trait_ref = self.resolve_vars_if_possible(data.parent_trait_pred); @@ -2392,14 +2392,14 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { obligated_types.push(ty); let parent_predicate = parent_trait_ref.to_predicate(tcx); - if !self.is_recursive_obligation(obligated_types, data.parent_code()) { + if !self.is_recursive_obligation(obligated_types, &data.parent_code) { // #74711: avoid a stack overflow ensure_sufficient_stack(|| { self.note_obligation_cause_code( err, &parent_predicate, param_env, - data.parent_code(), + &data.parent_code, obligated_types, seen_requirements, ) @@ -2410,7 +2410,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err, &parent_predicate, param_env, - &cause_code.peel_derives(), + cause_code.peel_derives(), obligated_types, seen_requirements, ) @@ -2461,7 +2461,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // We don't want to point at the ADT saying "required because it appears within // the type `X`", like we would otherwise do in test `supertrait-auto-trait.rs`. while let ObligationCauseCode::BuiltinDerivedObligation(derived) = - data.parent_code() + &*data.parent_code { let child_trait_ref = self.resolve_vars_if_possible(derived.parent_trait_pred); @@ -2474,7 +2474,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { parent_trait_pred = child_trait_ref; } } - while let ObligationCauseCode::ImplDerivedObligation(child) = data.parent_code() { + while let ObligationCauseCode::ImplDerivedObligation(child) = &*data.parent_code { // Skip redundant recursive obligation notes. See `ui/issue-20413.rs`. let child_trait_pred = self.resolve_vars_if_possible(child.derived.parent_trait_pred); @@ -2505,7 +2505,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err, &parent_predicate, param_env, - data.parent_code(), + &data.parent_code, obligated_types, seen_requirements, ) @@ -2520,7 +2520,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err, &parent_predicate, param_env, - data.parent_code(), + &data.parent_code, obligated_types, seen_requirements, ) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 300b87aa46575..7c180bd164322 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -1606,9 +1606,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut result_code = code; loop { let parent = match code { - ObligationCauseCode::ImplDerivedObligation(c) => c.derived.parent_code(), + ObligationCauseCode::ImplDerivedObligation(c) => &c.derived.parent_code, ObligationCauseCode::BuiltinDerivedObligation(c) - | ObligationCauseCode::DerivedObligation(c) => c.parent_code(), + | ObligationCauseCode::DerivedObligation(c) => &c.parent_code, _ => break result_code, }; (result_code, code) = (code, parent); @@ -1670,7 +1670,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { call_hir_id: expr.hir_id, parent_code, } - .into() }); } else if error.obligation.cause.span == call_sp { // Make function calls point at the callee, not the whole thing. From 72f144de242499776d8ca2f6626d63d169781a05 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 12 May 2022 11:29:01 +0000 Subject: [PATCH 13/15] Give the inliner some hints --- compiler/rustc_middle/src/traits/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 4b67fc84d2ed8..8072dd16ae826 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -133,6 +133,7 @@ impl<'tcx> ObligationCause<'tcx> { ObligationCause::dummy_with_span(DUMMY_SP) } + #[inline(always)] pub fn dummy_with_span(span: Span) -> ObligationCause<'tcx> { ObligationCause { span, body_id: hir::CRATE_HIR_ID, code: Default::default() } } @@ -203,6 +204,7 @@ pub struct InternedObligationCauseCode<'tcx> { } impl<'tcx> From> for InternedObligationCauseCode<'tcx> { + #[inline(always)] fn from(code: ObligationCauseCode<'tcx>) -> Self { Self { code: if code == MISC_OBLIGATION_CAUSE_CODE { None } else { Some(Lrc::new(code)) } } } From 59bbbe78e2ddf6f5c823372890b928fe19e41ac3 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 13 May 2022 09:30:09 +0000 Subject: [PATCH 14/15] Avoid invoking the full `eq` infrastructure when all we want is to check a discriminant --- compiler/rustc_middle/src/traits/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 8072dd16ae826..dcd457957a819 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -206,7 +206,9 @@ pub struct InternedObligationCauseCode<'tcx> { impl<'tcx> From> for InternedObligationCauseCode<'tcx> { #[inline(always)] fn from(code: ObligationCauseCode<'tcx>) -> Self { - Self { code: if code == MISC_OBLIGATION_CAUSE_CODE { None } else { Some(Lrc::new(code)) } } + Self { + code: if let MISC_OBLIGATION_CAUSE_CODE = code { None } else { Some(Lrc::new(code)) }, + } } } From 0cefa5fa183fc2ff672d68a4c67009b79ded76e4 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 16 May 2022 13:34:03 +0000 Subject: [PATCH 15/15] Force inline InternedObligationCauseCode creation --- compiler/rustc_middle/src/traits/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index dcd457957a819..04a4d07394565 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -203,11 +203,11 @@ pub struct InternedObligationCauseCode<'tcx> { code: Option>>, } -impl<'tcx> From> for InternedObligationCauseCode<'tcx> { +impl<'tcx> ObligationCauseCode<'tcx> { #[inline(always)] - fn from(code: ObligationCauseCode<'tcx>) -> Self { - Self { - code: if let MISC_OBLIGATION_CAUSE_CODE = code { None } else { Some(Lrc::new(code)) }, + fn into(self) -> InternedObligationCauseCode<'tcx> { + InternedObligationCauseCode { + code: if let MISC_OBLIGATION_CAUSE_CODE = self { None } else { Some(Lrc::new(self)) }, } } }