Skip to content

Commit

Permalink
Auto merge of rust-lang#118267 - compiler-errors:ambiguity-causes, r=…
Browse files Browse the repository at this point in the history 
…spastorino

`AmbiguityCause` should not eagerly format strings

Minor tweak found when working on some coherence diagnostics stuff (towards `-Ztrait-solver=next-coherence` stabilization)
  • Loading branch information
@bors
bors committed Nov 26, 2023
2 parents 3acb261 + 0750a5d commit 0b8a61b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 67 deletions.
39 changes: 15 additions & 24 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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 });
}
}
}
Expand Down Expand Up @@ -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,
}
}
});
Expand Down Expand Up @@ -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 });
}
80 changes: 38 additions & 42 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(),
})
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/specialize/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down

0 comments on commit 0b8a61b

Please sign in to comment.