Skip to content

AmbiguityCause should not eagerly format strings #118267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2023
Merged

AmbiguityCause should not eagerly format strings #118267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 15 additions & 24 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
View file
Open in desktop
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 @@ -979,8 +978,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 @@ -989,11 +988,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 @@ -1033,14 +1032,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 @@ -1078,24 +1075,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 @@ -1132,7 +1123,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
Open in desktop
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
Open in desktop
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