From 0750a5df4d1d04f95e117939493af4713fa71d2e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 24 Nov 2023 18:10:30 +0000 Subject: [PATCH] AmbiguityCause should not eagerly format strings --- .../src/traits/coherence.rs | 39 ++++----- .../src/traits/select/mod.rs | 84 +++++++++---------- .../src/traits/specialize/mod.rs | 2 +- 3 files changed, 56 insertions(+), 69 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 9119792e324..ed55d5267d1 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -29,7 +29,6 @@ use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::traits::DefiningAnchor; use rustc_middle::ty::fast_reject::{DeepRejectCtxt, TreatParams}; -use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt}; use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor}; use rustc_session::lint::builtin::COINDUCTIVE_OVERLAP_IN_COHERENCE; @@ -54,7 +53,7 @@ pub enum Conflict { pub struct OverlapResult<'tcx> { pub impl_header: ty::ImplHeader<'tcx>, - pub intercrate_ambiguity_causes: FxIndexSet, + pub intercrate_ambiguity_causes: FxIndexSet>, /// `true` if the overlap might've been permitted before the shift /// to universes. @@ -979,8 +978,8 @@ fn visit_const(&mut self, _c: ty::Const<'tcx>) -> ControlFlow { fn compute_intercrate_ambiguity_causes<'tcx>( infcx: &InferCtxt<'tcx>, obligations: &[PredicateObligation<'tcx>], -) -> FxIndexSet { - let mut causes: FxIndexSet = Default::default(); +) -> FxIndexSet> { + let mut causes: FxIndexSet> = Default::default(); for obligation in obligations { search_ambiguity_causes(infcx, obligation.clone().into(), &mut causes); @@ -989,11 +988,11 @@ fn compute_intercrate_ambiguity_causes<'tcx>( causes } -struct AmbiguityCausesVisitor<'a> { - causes: &'a mut FxIndexSet, +struct AmbiguityCausesVisitor<'a, 'tcx> { + causes: &'a mut FxIndexSet>, } -impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a> { +impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a, 'tcx> { type BreakTy = !; fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow { let infcx = goal.infcx(); @@ -1033,14 +1032,12 @@ fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow) -> ControlFlow { if !trait_ref.references_error() { let self_ty = trait_ref.self_ty(); - let (trait_desc, self_desc) = with_no_trimmed_paths!({ - let trait_desc = trait_ref.print_only_trait_path().to_string(); - let self_desc = self_ty - .has_concrete_skeleton() - .then(|| self_ty.to_string()); - (trait_desc, self_desc) - }); + let self_ty = self_ty.has_concrete_skeleton().then(|| self_ty); ambiguity_cause = Some(match conflict { Conflict::Upstream => { IntercrateAmbiguityCause::UpstreamCrateUpdate { - trait_desc, - self_desc, + trait_ref, + self_ty, } } Conflict::Downstream => { IntercrateAmbiguityCause::DownstreamCrate { - trait_desc, - self_desc, + trait_ref, + self_ty, } } }); @@ -1132,7 +1123,7 @@ fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow( infcx: &InferCtxt<'tcx>, goal: Goal<'tcx, ty::Predicate<'tcx>>, - causes: &mut FxIndexSet, + causes: &mut FxIndexSet>, ) { infcx.visit_proof_tree(goal, &mut AmbiguityCausesVisitor { causes }); } diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index f3e6887e012..0c884b7c16a 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -46,6 +46,7 @@ use rustc_middle::ty::{self, EarlyBinder, PolyProjectionPredicate, ToPolyTraitRef, ToPredicate}; use rustc_middle::ty::{Ty, TyCtxt, TypeFoldable, TypeVisitableExt}; use rustc_span::symbol::sym; +use rustc_span::Symbol; use std::cell::{Cell, RefCell}; use std::cmp; @@ -59,13 +60,13 @@ mod confirmation; #[derive(Clone, Debug, Eq, PartialEq, Hash)] -pub enum IntercrateAmbiguityCause { - DownstreamCrate { trait_desc: String, self_desc: Option }, - UpstreamCrateUpdate { trait_desc: String, self_desc: Option }, - ReservationImpl { message: String }, +pub enum IntercrateAmbiguityCause<'tcx> { + DownstreamCrate { trait_ref: ty::TraitRef<'tcx>, self_ty: Option> }, + UpstreamCrateUpdate { trait_ref: ty::TraitRef<'tcx>, self_ty: Option> }, + ReservationImpl { message: Symbol }, } -impl IntercrateAmbiguityCause { +impl<'tcx> IntercrateAmbiguityCause<'tcx> { /// Emits notes when the overlap is caused by complex intercrate ambiguities. /// See #23980 for details. pub fn add_intercrate_ambiguity_hint(&self, err: &mut Diagnostic) { @@ -73,28 +74,32 @@ pub fn add_intercrate_ambiguity_hint(&self, err: &mut Diagnostic) { } pub fn intercrate_ambiguity_hint(&self) -> String { - match self { - IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc } => { - let self_desc = if let Some(ty) = self_desc { - format!(" for type `{ty}`") - } else { - String::new() - }; - format!("downstream crates may implement trait `{trait_desc}`{self_desc}") - } - IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc } => { - let self_desc = if let Some(ty) = self_desc { - format!(" for type `{ty}`") - } else { - String::new() - }; + with_no_trimmed_paths!(match self { + IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty } => { format!( - "upstream crates may add a new impl of trait `{trait_desc}`{self_desc} \ - in future versions" + "downstream crates may implement trait `{trait_desc}`{self_desc}", + trait_desc = trait_ref.print_only_trait_path(), + self_desc = if let Some(self_ty) = self_ty { + format!(" for type `{self_ty}`") + } else { + String::new() + } ) } - IntercrateAmbiguityCause::ReservationImpl { message } => message.clone(), - } + IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_ref, self_ty } => { + format!( + "upstream crates may add a new impl of trait `{trait_desc}`{self_desc} \ + in future versions", + trait_desc = trait_ref.print_only_trait_path(), + self_desc = if let Some(self_ty) = self_ty { + format!(" for type `{self_ty}`") + } else { + String::new() + } + ) + } + IntercrateAmbiguityCause::ReservationImpl { message } => message.to_string(), + }) } } @@ -114,7 +119,7 @@ pub struct SelectionContext<'cx, 'tcx> { /// We don't do his until we detect a coherence error because it can /// lead to false overflow results (#47139) and because always /// computing it may negatively impact performance. - intercrate_ambiguity_causes: Option>, + intercrate_ambiguity_causes: Option>>, /// The mode that trait queries run in, which informs our error handling /// policy. In essence, canonicalized queries need their errors propagated @@ -270,7 +275,9 @@ pub fn enable_tracking_intercrate_ambiguity_causes(&mut self) { /// Gets the intercrate ambiguity causes collected since tracking /// was enabled and disables tracking at the same time. If /// tracking is not enabled, just returns an empty vector. - pub fn take_intercrate_ambiguity_causes(&mut self) -> FxIndexSet { + pub fn take_intercrate_ambiguity_causes( + &mut self, + ) -> FxIndexSet> { assert!(self.is_intercrate()); self.intercrate_ambiguity_causes.take().unwrap_or_default() } @@ -428,19 +435,11 @@ fn candidate_from_obligation_no_cache<'o>( ); if !trait_ref.references_error() { let self_ty = trait_ref.self_ty(); - let (trait_desc, self_desc) = with_no_trimmed_paths!({ - let trait_desc = trait_ref.print_only_trait_path().to_string(); - let self_desc = - self_ty.has_concrete_skeleton().then(|| self_ty.to_string()); - (trait_desc, self_desc) - }); + let self_ty = self_ty.has_concrete_skeleton().then(|| self_ty); let cause = if let Conflict::Upstream = conflict { - IntercrateAmbiguityCause::UpstreamCrateUpdate { - trait_desc, - self_desc, - } + IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_ref, self_ty } } else { - IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc } + IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty } }; debug!(?cause, "evaluate_stack: pushing cause"); self.intercrate_ambiguity_causes.as_mut().unwrap().insert(cause); @@ -1451,20 +1450,17 @@ fn filter_reservation_impls( if let ImplCandidate(def_id) = candidate { if let ty::ImplPolarity::Reservation = tcx.impl_polarity(def_id) { if let Some(intercrate_ambiguity_clauses) = &mut self.intercrate_ambiguity_causes { - let value = tcx + let message = tcx .get_attr(def_id, sym::rustc_reservation_impl) .and_then(|a| a.value_str()); - if let Some(value) = value { + if let Some(message) = message { debug!( "filter_reservation_impls: \ reservation impl ambiguity on {:?}", def_id ); - intercrate_ambiguity_clauses.insert( - IntercrateAmbiguityCause::ReservationImpl { - message: value.to_string(), - }, - ); + intercrate_ambiguity_clauses + .insert(IntercrateAmbiguityCause::ReservationImpl { message }); } } return Ok(None); diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs index 0f7a0ab17ef..a3f6470fd37 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs @@ -37,7 +37,7 @@ pub struct OverlapError<'tcx> { pub with_impl: DefId, pub trait_ref: ty::TraitRef<'tcx>, pub self_ty: Option>, - pub intercrate_ambiguity_causes: FxIndexSet, + pub intercrate_ambiguity_causes: FxIndexSet>, pub involves_placeholder: bool, }