Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions compiler/rustc_hir_typeck/src/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
})
}

probe::TraitPick(_) => {
probe::TraitPick { .. } => {
let trait_def_id = pick.item.container_id(self.tcx);

// Make a trait reference `$0 : Trait<$1...$n>`
Expand Down Expand Up @@ -756,7 +756,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
pick: &probe::Pick<'_>,
segment: &hir::PathSegment<'tcx>,
) {
if pick.kind != probe::PickKind::TraitPick(true) {
if pick.kind != (probe::PickKind::TraitPick { is_ambiguously_imported: true }) {
return;
}
let trait_name = self.tcx.item_name(pick.item.container_id(self.tcx));
Expand All @@ -767,11 +767,11 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
segment.hir_id,
rustc_errors::DiagDecorator(|diag| {
diag.primary_message(format!(
"Use of ambiguously glob imported trait `{trait_name}`"
"use of ambiguously glob imported trait `{trait_name}`"
))
.span(segment.ident.span)
.span_label(import_span, format!("`{trait_name}` imported ambiguously here"))
.help(format!("Import `{trait_name}` explicitly"));
.help(format!("import `{trait_name}` explicitly"));
}),
);
}
Expand Down
53 changes: 30 additions & 23 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub(crate) struct Candidate<'tcx> {
pub(crate) enum CandidateKind<'tcx> {
InherentImplCandidate { impl_def_id: DefId, receiver_steps: usize },
ObjectCandidate(ty::PolyTraitRef<'tcx>),
TraitCandidate(ty::PolyTraitRef<'tcx>, bool /* lint_ambiguous */),
TraitCandidate { trait_ref: ty::PolyTraitRef<'tcx>, is_ambiguously_imported: bool },
WhereClauseCandidate(ty::PolyTraitRef<'tcx>),
}

Expand Down Expand Up @@ -239,18 +239,17 @@ pub(crate) struct Pick<'tcx> {
/// Only applies for inherent impls.
pub receiver_steps: Option<usize>,

/// Candidates that were shadowed by supertraits.
/// Candidates that were shadowed by subtraits.
pub shadowed_candidates: Vec<ty::AssocItem>,
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum PickKind<'tcx> {
InherentImplPick,
ObjectPick,
TraitPick(
// Is Ambiguously Imported
bool,
),
TraitPick {
is_ambiguously_imported: bool,
},
WhereClausePick(
// Trait
ty::PolyTraitRef<'tcx>,
Expand Down Expand Up @@ -611,10 +610,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Candidate {
item,
kind: match item.container {
AssocContainer::Trait => CandidateKind::TraitCandidate(
ty::Binder::dummy(trait_ref),
false,
),
AssocContainer::Trait => CandidateKind::TraitCandidate {
trait_ref: ty::Binder::dummy(trait_ref),
is_ambiguously_imported: false,
},
AssocContainer::InherentImpl => {
CandidateKind::InherentImplCandidate {
impl_def_id: self.tcx.parent(def_id),
Expand Down Expand Up @@ -1143,7 +1142,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
&mut self,
import_ids: &'tcx [LocalDefId],
trait_def_id: DefId,
lint_ambiguous: bool,
is_ambiguously_imported: bool,
) {
let trait_args = self.fresh_args_for_item(self.span, trait_def_id);
let trait_ref = ty::TraitRef::new_from_args(self.tcx, trait_def_id, trait_args);
Expand All @@ -1165,7 +1164,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
Candidate {
item,
import_ids,
kind: TraitCandidate(bound_trait_ref, lint_ambiguous),
kind: TraitCandidate {
trait_ref: bound_trait_ref,
is_ambiguously_imported,
},
},
false,
);
Expand All @@ -1188,7 +1190,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
Candidate {
item,
import_ids,
kind: TraitCandidate(ty::Binder::dummy(trait_ref), lint_ambiguous),
kind: TraitCandidate {
trait_ref: ty::Binder::dummy(trait_ref),
is_ambiguously_imported,
},
},
false,
);
Expand Down Expand Up @@ -1958,7 +1963,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
ObjectCandidate(_) | WhereClauseCandidate(_) => {
CandidateSource::Trait(candidate.item.container_id(self.tcx))
}
TraitCandidate(trait_ref, _) => self.probe(|_| {
TraitCandidate { trait_ref, is_ambiguously_imported: _ } => self.probe(|_| {
let trait_ref = self.instantiate_binder_with_fresh_vars(
self.span,
BoundRegionConversionTime::FnCall,
Expand Down Expand Up @@ -1988,7 +1993,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
fn candidate_source_from_pick(&self, pick: &Pick<'tcx>) -> CandidateSource {
match pick.kind {
InherentImplPick => CandidateSource::Impl(pick.item.container_id(self.tcx)),
ObjectPick | WhereClausePick(_) | TraitPick(_) => {
ObjectPick | WhereClausePick(_) | TraitPick { .. } => {
CandidateSource::Trait(pick.item.container_id(self.tcx))
}
}
Expand Down Expand Up @@ -2069,7 +2074,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
impl_bounds,
));
}
TraitCandidate(poly_trait_ref, _) => {
TraitCandidate { trait_ref: poly_trait_ref, is_ambiguously_imported: _ } => {
// Some trait methods are excluded for arrays before 2021.
// (`array.into_iter()` wants a slice iterator for compatibility.)
if let Some(method_name) = self.method_name {
Expand Down Expand Up @@ -2373,16 +2378,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}

// They are all the same, so if any of them is ambiguous, we report the pick as ambiguous.
let lint_ambiguous = probes.iter().any(|(p, _)| match p.kind {
TraitCandidate(_, lint) => lint,
let is_ambiguously_imported = probes.iter().any(|(p, _)| match p.kind {
TraitCandidate { is_ambiguously_imported, .. } => is_ambiguously_imported,
_ => false,
});

// FIXME: check the return type here somehow.
// If so, just use this trait and call it a day.
Some(Pick {
item: probes[0].0.item,
kind: TraitPick(lint_ambiguous),
kind: TraitPick { is_ambiguously_imported },
import_ids: probes[0].0.import_ids,
autoderefs: 0,
autoref_or_ptr_adjustment: None,
Expand Down Expand Up @@ -2457,14 +2462,14 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}
}

let lint_ambiguous = match child_candidate.kind {
TraitCandidate(_, lint) => lint,
let is_ambiguously_imported = match child_candidate.kind {
TraitCandidate { is_ambiguously_imported, .. } => is_ambiguously_imported,
_ => false,
};

Some(Pick {
item: child_candidate.item,
kind: TraitPick(lint_ambiguous),
kind: TraitPick { is_ambiguously_imported },
import_ids: child_candidate.import_ids,
autoderefs: 0,
autoref_or_ptr_adjustment: None,
Expand Down Expand Up @@ -2712,7 +2717,9 @@ impl<'tcx> Candidate<'tcx> {
kind: match self.kind {
InherentImplCandidate { .. } => InherentImplPick,
ObjectCandidate(_) => ObjectPick,
TraitCandidate(_, lint_ambiguous) => TraitPick(lint_ambiguous),
TraitCandidate { is_ambiguously_imported, .. } => {
TraitPick { is_ambiguously_imported }
}
WhereClauseCandidate(trait_ref) => {
// Only trait derived from where-clauses should
// appear here, so they should not contain any
Expand Down
99 changes: 24 additions & 75 deletions compiler/rustc_trait_selection/src/solve/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use tracing::instrument;
use self::derive_errors::*;
use super::Certainty;
use super::delegate::SolverDelegate;
use crate::error_reporting::InferCtxtErrorExt;
use crate::traits::{FulfillmentError, FulfillmentErrorCode, ScrubbedTraitError};

mod derive_errors;
Expand Down Expand Up @@ -53,12 +54,6 @@ pub struct FulfillmentCtxt<'tcx, E: 'tcx> {

#[derive(Default, Debug)]
struct ObligationStorage<'tcx> {
/// Obligations which resulted in an overflow in fulfillment itself.
///
/// We cannot eagerly return these as error so we instead store them here
/// to avoid recomputing them each time `try_evaluate_obligations` is called.
/// This also allows us to return the correct `FulfillmentError` for them.
overflowed: Vec<PredicateObligation<'tcx>>,
pending: PendingObligations<'tcx>,
}

Expand All @@ -72,24 +67,18 @@ impl<'tcx> ObligationStorage<'tcx> {
}

fn has_pending_obligations(&self) -> bool {
!self.pending.is_empty() || !self.overflowed.is_empty()
!self.pending.is_empty()
}

fn clone_pending(&self) -> PredicateObligations<'tcx> {
let mut obligations: PredicateObligations<'tcx> =
self.pending.iter().map(|(o, _)| o.clone()).collect();
obligations.extend(self.overflowed.iter().cloned());
obligations
self.pending.iter().map(|(o, _)| o.clone()).collect()
}

fn clone_pending_filtered<F>(&self, f: F) -> PredicateObligations<'tcx>
where
F: FnMut(&&(PredicateObligation<'tcx>, Option<GoalStalledOn<TyCtxt<'tcx>>>)) -> bool,
{
let mut obligations: PredicateObligations<'tcx> =
self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect();
obligations.extend(self.overflowed.iter().cloned());
obligations
self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect()
}

fn drain_pending(
Expand All @@ -101,29 +90,6 @@ impl<'tcx> ObligationStorage<'tcx> {
self.pending = pending;
unstalled
}

fn on_fulfillment_overflow(&mut self, infcx: &InferCtxt<'tcx>) {
infcx.probe(|_| {
// IMPORTANT: we must not use solve any inference variables in the obligations
// as this is all happening inside of a probe. We use a probe to make sure
// we get all obligations involved in the overflow. We pretty much check: if
// we were to do another step of `try_evaluate_obligations`, which goals would
// change.
self.overflowed.extend(
self.pending
.extract_if(.., |(o, stalled_on)| {
let goal = o.as_goal();
let result = <&SolverDelegate<'tcx>>::from(infcx).evaluate_root_goal(
goal,
o.cause.span,
stalled_on.take(),
);
matches!(result, Ok(GoalEvaluation { has_changed: HasChanged::Yes, .. }))
})
.map(|(o, _)| o),
);
})
}
}

impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> {
Expand Down Expand Up @@ -186,7 +152,7 @@ where

#[inline]
fn collect_remaining_errors(&mut self, infcx: &InferCtxt<'tcx>) -> TraitErrors<E> {
if self.obligations.pending.is_empty() && self.obligations.overflowed.is_empty() {
if self.obligations.pending.is_empty() {
// Typically in more than 99.9% of cases this condition is true, therefore we outline
// the other case.
TraitErrors::NoErrors
Expand All @@ -202,13 +168,8 @@ where
let delegate = <&SolverDelegate<'tcx>>::from(infcx);
loop {
let mut any_changed = false;
let mut overflowed = false;

self.obligations.pending.retain_mut(|(obligation, opt_stalled_on)| {
if overflowed {
return false;
}

// Common case: still stalled; keep the obligation. This path is extremely hot in
// some cases; there can be thousands of pending obligations.
if let Some(stalled_on) = opt_stalled_on
Expand Down Expand Up @@ -239,21 +200,26 @@ where
// constrained by evaluating the goal.
obligation.predicate = goal.predicate;
if has_changed == HasChanged::Yes {
// We increment the recursion depth here to track the number of times
// this goal has resulted in inference progress. This doesn't precisely
// model the way that we track recursion depth in the old solver due
// to the fact that we only process root obligations, but it is a good
// approximation and should only result in fulfillment overflow in
// pathological cases.
obligation.recursion_depth += 1;

if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) {
// At this point we want to stop evaluating goals. We can't break out of
// `retain_mut`, so instead we set this flag which causes all other
// elements to be skipped.
overflowed = true;
return false;
// We limit the total count of inference progress to avoid hang so we don't
// try to recover from this.
// It's more complicated to collect all overflows thus we stopped doing that.
// Eager aborting is also what the old solver does.
//
// Note: it's incredibly rare to actually encounter fulfillment overflow
// as a single obligation would have to result in different inference progress
// a `recursion_depth` number of times. This mostly happens in bugs or with
// `Subtype` obligations because we no longer use the `sub_unification_table`
// in generalization.
infcx.err_ctxt().report_overflow_obligation(obligation, true);
} else {
// We increment the recursion depth here to track the number of times
// this goal has resulted in inference progress. This doesn't precisely
// model the way that we track recursion depth in the old solver due
// to the fact that we only process root obligations, but it is a good
// approximation and should only result in fulfillment overflow in
// pathological cases.
obligation.recursion_depth += 1;
any_changed = true;
}
}
Expand Down Expand Up @@ -288,11 +254,6 @@ where
}
}
});
if overflowed {
self.obligations.on_fulfillment_overflow(infcx);
// Only return true errors that we have accumulated while processing.
return errors;
}

if !any_changed {
break;
Expand Down Expand Up @@ -409,12 +370,6 @@ where
.filter_map(|(obligation, _)| {
try_ambiguity_error_for_stalled(infcx, obligation).map(NextSolverError::Ambiguity)
})
.chain(
cx.obligations
.overflowed
.drain(..)
.map(|obligation| NextSolverError::Overflow(obligation)),
)
.map(|e| E::from_solver_error(infcx, e))
.collect()
}
Expand All @@ -432,7 +387,6 @@ pub struct NextSolverAmbiguityError<'tcx> {
pub enum NextSolverError<'tcx> {
TrueError(PredicateObligation<'tcx>),
Ambiguity(NextSolverAmbiguityError<'tcx>),
Overflow(PredicateObligation<'tcx>),
}

impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tcx> {
Expand All @@ -444,9 +398,6 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tc
NextSolverError::Ambiguity(ambiguity) => {
fulfillment_error_for_stalled(infcx, ambiguity)
}
NextSolverError::Overflow(obligation) => {
fulfillment_error_for_overflow(infcx, obligation)
}
}
}
}
Expand All @@ -455,9 +406,7 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for ScrubbedTraitError<'
fn from_solver_error(_infcx: &InferCtxt<'tcx>, error: NextSolverError<'tcx>) -> Self {
match error {
NextSolverError::TrueError(_) => ScrubbedTraitError::TrueError,
NextSolverError::Ambiguity(_) | NextSolverError::Overflow(_) => {
ScrubbedTraitError::Ambiguity
}
NextSolverError::Ambiguity(_) => ScrubbedTraitError::Ambiguity,
}
}
}
Expand Down
Loading
Loading