Auto merge of #118267 - compiler-errors:ambiguity-causes, r=spastorino

`AmbiguityCause` should not eagerly format strings

Minor tweak found when working on some coherence diagnostics stuff (towards `-Ztrait-solver=next-coherence` stabilization)

Author: bors

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -29,7 +29,6 @@ use rustc_middle::traits::solve::{CandidateSource, Certainty, Goal};
 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<IntercrateAmbiguityCause>,
+    pub intercrate_ambiguity_causes: FxIndexSet<IntercrateAmbiguityCause<'tcx>>,
 
     /// `true` if the overlap might've been permitted before the shift
     /// to universes.
@@ -987,8 +986,8 @@ where
 fn compute_intercrate_ambiguity_causes<'tcx>(
     infcx: &InferCtxt<'tcx>,
     obligations: &[PredicateObligation<'tcx>],
-) -> FxIndexSet<IntercrateAmbiguityCause> {
-    let mut causes: FxIndexSet<IntercrateAmbiguityCause> = Default::default();
+) -> FxIndexSet<IntercrateAmbiguityCause<'tcx>> {
+    let mut causes: FxIndexSet<IntercrateAmbiguityCause<'tcx>> = Default::default();
 
     for obligation in obligations {
         search_ambiguity_causes(infcx, obligation.clone().into(), &mut causes);
@@ -997,11 +996,11 @@ fn compute_intercrate_ambiguity_causes<'tcx>(
     causes
 }
 
-struct AmbiguityCausesVisitor<'a> {
-    causes: &'a mut FxIndexSet<IntercrateAmbiguityCause>,
+struct AmbiguityCausesVisitor<'a, 'tcx> {
+    causes: &'a mut FxIndexSet<IntercrateAmbiguityCause<'tcx>>,
 }
 
-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<Self::BreakTy> {
         let infcx = goal.infcx();
@@ -1041,14 +1040,12 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a> {
             } = cand.kind()
             {
                 if let ty::ImplPolarity::Reservation = infcx.tcx.impl_polarity(def_id) {
-                    let value = infcx
+                    let message = infcx
                         .tcx
                         .get_attr(def_id, sym::rustc_reservation_impl)
                         .and_then(|a| a.value_str());
-                    if let Some(value) = value {
-                        self.causes.insert(IntercrateAmbiguityCause::ReservationImpl {
-                            message: value.to_string(),
-                        });
+                    if let Some(message) = message {
+                        self.causes.insert(IntercrateAmbiguityCause::ReservationImpl { message });
                     }
                 }
             }
@@ -1088,24 +1085,18 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a> {
                         Ok(Err(conflict)) => {
                             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,
                                         }
                                     }
                                 });
@@ -1142,7 +1133,7 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for AmbiguityCausesVisitor<'a> {
 fn search_ambiguity_causes<'tcx>(
     infcx: &InferCtxt<'tcx>,
     goal: Goal<'tcx, ty::Predicate<'tcx>>,
-    causes: &mut FxIndexSet<IntercrateAmbiguityCause>,
+    causes: &mut FxIndexSet<IntercrateAmbiguityCause<'tcx>>,
 ) {
     infcx.visit_proof_tree(goal, &mut AmbiguityCausesVisitor { causes });
 }

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -46,6 +46,7 @@ use rustc_middle::ty::GenericArgsRef;
 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,42 +60,46 @@ mod candidate_assembly;
 mod confirmation;
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
-pub enum IntercrateAmbiguityCause {
-    DownstreamCrate { trait_desc: String, self_desc: Option<String> },
-    UpstreamCrateUpdate { trait_desc: String, self_desc: Option<String> },
-    ReservationImpl { message: String },
+pub enum IntercrateAmbiguityCause<'tcx> {
+    DownstreamCrate { trait_ref: ty::TraitRef<'tcx>, self_ty: Option<Ty<'tcx>> },
+    UpstreamCrateUpdate { trait_ref: ty::TraitRef<'tcx>, self_ty: Option<Ty<'tcx>> },
+    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) {
         err.note(self.intercrate_ambiguity_hint());
     }
 
     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}")
+        with_no_trimmed_paths!(match self {
+            IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty } => {
+                format!(
+                    "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::UpstreamCrateUpdate { trait_desc, self_desc } => {
-                let self_desc = if let Some(ty) = self_desc {
-                    format!(" for type `{ty}`")
-                } else {
-                    String::new()
-                };
+            IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_ref, self_ty } => {
                 format!(
                     "upstream crates may add a new impl of trait `{trait_desc}`{self_desc} \
-                     in future versions"
+                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.clone(),
-        }
+            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<FxIndexSet<IntercrateAmbiguityCause>>,
+    intercrate_ambiguity_causes: Option<FxIndexSet<IntercrateAmbiguityCause<'tcx>>>,
 
     /// 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 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// 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<IntercrateAmbiguityCause> {
+    pub fn take_intercrate_ambiguity_causes(
+        &mut self,
+    ) -> FxIndexSet<IntercrateAmbiguityCause<'tcx>> {
         assert!(self.is_intercrate());
         self.intercrate_ambiguity_causes.take().unwrap_or_default()
     }
@@ -428,19 +435,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                         );
                         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 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         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);

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<Ty<'tcx>>,
-    pub intercrate_ambiguity_causes: FxIndexSet<IntercrateAmbiguityCause>,
+    pub intercrate_ambiguity_causes: FxIndexSet<IntercrateAmbiguityCause<'tcx>>,
     pub involves_placeholder: bool,
 }