Skip to content
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.

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

selection failure: recompute applicable impls #103252

Merged
merged 3 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
selection failure: recompute applicable impls
  • Loading branch information
lcnr committed Nov 8, 2022
commit f1551bfc02845eb198a71cc5c0264bd71e336274
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,6 @@ pub enum SelectionError<'tcx> {
/// Signaling that an error has already been emitted, to avoid
/// multiple errors being shown.
ErrorReporting,
/// Multiple applicable `impl`s where found. The `DefId`s correspond to
/// all the `impl`s' Items.
Ambiguous(Vec<DefId>),
}

/// When performing resolution, it is typically the case that there
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2550,11 +2550,11 @@ impl<'tcx> TyCtxt<'tcx> {

/// Looks up the span of `impl_did` if the impl is local; otherwise returns `Err`
/// with the name of the crate containing the impl.
pub fn span_of_impl(self, impl_did: DefId) -> Result<Span, Symbol> {
if let Some(impl_did) = impl_did.as_local() {
Ok(self.def_span(impl_did))
pub fn span_of_impl(self, impl_def_id: DefId) -> Result<Span, Symbol> {
if let Some(impl_def_id) = impl_def_id.as_local() {
Ok(self.def_span(impl_def_id))
} else {
Err(self.crate_name(impl_did.krate))
Err(self.crate_name(impl_def_id.krate))
}
}

Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,22 @@ pub struct FulfillmentContext<'tcx> {
obligations: FxIndexSet<PredicateObligation<'tcx>>,

relationships: FxHashMap<ty::TyVid, ty::FoundRelationships>,

usable_in_snapshot: bool,
}

impl FulfillmentContext<'_> {
pub(crate) fn new() -> Self {
FulfillmentContext {
obligations: FxIndexSet::default(),
relationships: FxHashMap::default(),
usable_in_snapshot: false,
}
}

pub(crate) fn new_in_snapshot() -> Self {
FulfillmentContext { usable_in_snapshot: true, ..Self::new() }
}
}

impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
Expand All @@ -41,7 +48,9 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
infcx: &InferCtxt<'tcx>,
obligation: PredicateObligation<'tcx>,
) {
assert!(!infcx.is_in_snapshot());
if !self.usable_in_snapshot {
jackh726 marked this conversation as resolved.
Show resolved Hide resolved
assert!(!infcx.is_in_snapshot());
}
let obligation = infcx.resolve_vars_if_possible(obligation);

super::relationships::update(self, infcx, &obligation);
Expand Down Expand Up @@ -72,7 +81,9 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
}

fn select_where_possible(&mut self, infcx: &InferCtxt<'tcx>) -> Vec<FulfillmentError<'tcx>> {
assert!(!infcx.is_in_snapshot());
if !self.usable_in_snapshot {
assert!(!infcx.is_in_snapshot());
}

let mut errors = Vec::new();
let mut next_round = FxIndexSet::default();
Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_trait_selection/src/traits/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<'tcx> TraitEngineExt<'tcx> for dyn TraitEngine<'tcx> {

fn new_in_snapshot(tcx: TyCtxt<'tcx>) -> Box<Self> {
if tcx.sess.opts.unstable_opts.chalk {
Box::new(ChalkFulfillmentContext::new())
Box::new(ChalkFulfillmentContext::new_in_snapshot())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for the changes here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, because ObligationCtxt::new_in_snapshot is used in fn recompute_ambiguous_candidates inside of a snapshot

} else {
Box::new(FulfillmentContext::new_in_snapshot())
}
Expand Down Expand Up @@ -119,13 +119,10 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> {
expected: T,
actual: T,
) -> Result<(), TypeError<'tcx>> {
match self.infcx.at(cause, param_env).eq(expected, actual) {
Ok(InferOk { obligations, value: () }) => {
self.register_obligations(obligations);
Ok(())
}
Err(e) => Err(e),
}
self.infcx
.at(cause, param_env)
.eq(expected, actual)
.map(|infer_ok| self.register_infer_ok_obligations(infer_ok))
}

pub fn sup<T: ToTrace<'tcx>>(
Expand All @@ -144,6 +141,10 @@ impl<'a, 'tcx> ObligationCtxt<'a, 'tcx> {
}
}

pub fn select_where_possible(&self) -> Vec<FulfillmentError<'tcx>> {
self.engine.borrow_mut().select_where_possible(self.infcx)
}

pub fn select_all_or_error(&self) -> Vec<FulfillmentError<'tcx>> {
self.engine.borrow_mut().select_all_or_error(self.infcx)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use rustc_hir::def_id::DefId;
use rustc_infer::infer::InferCtxt;
use rustc_infer::traits::{Obligation, ObligationCause, TraitObligation};
use rustc_span::DUMMY_SP;

use crate::traits::ObligationCtxt;

pub fn recompute_applicable_impls<'tcx>(
infcx: &InferCtxt<'tcx>,
obligation: &TraitObligation<'tcx>,
) -> Vec<DefId> {
let tcx = infcx.tcx;
let param_env = obligation.param_env;
let dummy_cause = ObligationCause::dummy();
let impl_may_apply = |impl_def_id| {
let ocx = ObligationCtxt::new_in_snapshot(infcx);
let placeholder_obligation =
infcx.replace_bound_vars_with_placeholders(obligation.predicate);
let obligation_trait_ref =
ocx.normalize(dummy_cause.clone(), param_env, placeholder_obligation.trait_ref);

let impl_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl_def_id);
let impl_trait_ref = tcx.bound_impl_trait_ref(impl_def_id).unwrap().subst(tcx, impl_substs);
let impl_trait_ref = ocx.normalize(ObligationCause::dummy(), param_env, impl_trait_ref);

if let Err(_) = ocx.eq(&dummy_cause, param_env, obligation_trait_ref, impl_trait_ref) {
return false;
}

let impl_predicates = tcx.predicates_of(impl_def_id).instantiate(tcx, impl_substs);
ocx.register_obligations(
impl_predicates
.predicates
.iter()
.map(|&predicate| Obligation::new(dummy_cause.clone(), param_env, predicate)),
);

ocx.select_where_possible().is_empty()
};

let mut impls = Vec::new();
tcx.for_each_relevant_impl(
obligation.predicate.def_id(),
obligation.predicate.skip_binder().trait_ref.self_ty(),
|impl_def_id| {
if infcx.probe(move |_snapshot| impl_may_apply(impl_def_id)) {
impls.push(impl_def_id)
}
},
);
impls
}
38 changes: 19 additions & 19 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod ambiguity;
pub mod on_unimplemented;
pub mod suggestions;

Expand Down Expand Up @@ -535,15 +536,6 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
let mut span = obligation.cause.span;

let mut err = match *error {
SelectionError::Ambiguous(ref impls) => {
let mut err = self.tcx.sess.struct_span_err(
obligation.cause.span,
&format!("multiple applicable `impl`s for `{}`", obligation.predicate),
);
self.annotate_source_of_ambiguity(&mut err, impls, obligation.predicate);
err.emit();
return;
}
SelectionError::Unimplemented => {
// If this obligation was generated as a result of well-formedness checking, see if we
// can get a better error message by performing HIR-based well-formedness checking.
Expand Down Expand Up @@ -2144,8 +2136,21 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
crate::traits::TraitQueryMode::Standard,
);
match selcx.select_from_obligation(&obligation) {
Err(SelectionError::Ambiguous(impls)) if impls.len() > 1 => {
self.annotate_source_of_ambiguity(&mut err, &impls, predicate);
Ok(None) => {
let impls = ambiguity::recompute_applicable_impls(self.infcx, &obligation);
let has_non_region_infer =
trait_ref.skip_binder().substs.types().any(|t| !t.is_ty_infer());
// It doesn't make sense to talk about applicable impls if there are more
// than a handful of them.
if impls.len() > 1 && impls.len() < 5 && has_non_region_infer {
self.annotate_source_of_ambiguity(&mut err, &impls, predicate);
} else {
if self.is_tainted_by_errors() {
err.cancel();
return;
}
err.note(&format!("cannot satisfy `{}`", predicate));
}
}
_ => {
if self.is_tainted_by_errors() {
Expand Down Expand Up @@ -2441,7 +2446,6 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
}
}
let msg = format!("multiple `impl`s satisfying `{}` found", predicate);
let mut crate_names: Vec<_> = crates.iter().map(|n| format!("`{}`", n)).collect();
crate_names.sort();
crate_names.dedup();
Expand All @@ -2462,13 +2466,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err.downgrade_to_delayed_bug();
return;
}
let post = if post.len() > 4 {
format!(
":\n{}\nand {} more",
post.iter().map(|p| format!("- {}", p)).take(4).collect::<Vec<_>>().join("\n"),
post.len() - 4,
)
} else if post.len() > 1 || (post.len() == 1 && post[0].contains('\n')) {

let msg = format!("multiple `impl`s satisfying `{}` found", predicate);
let post = if post.len() > 1 || (post.len() == 1 && post[0].contains('\n')) {
format!(":\n{}", post.iter().map(|p| format!("- {}", p)).collect::<Vec<_>>().join("\n"),)
} else if post.len() == 1 {
format!(": `{}`", post[0])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::traits;
use crate::traits::coherence::Conflict;
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::{util, SelectionResult};
use crate::traits::{Ambiguous, ErrorReporting, Overflow, Unimplemented};
use crate::traits::{ErrorReporting, Overflow, Unimplemented};

use super::BuiltinImplConditions;
use super::IntercrateAmbiguityCause;
Expand Down Expand Up @@ -200,15 +200,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// and report ambiguity.
if i > 1 {
debug!("multiple matches, ambig");
return Err(Ambiguous(
candidates
.into_iter()
.filter_map(|c| match c.candidate {
SelectionCandidate::ImplCandidate(def_id) => Some(def_id),
_ => None,
})
.collect(),
));
return Ok(None);
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
assert!(self.query_mode == TraitQueryMode::Canonical);
return Err(SelectionError::Overflow(OverflowError::Canonical));
}
Err(SelectionError::Ambiguous(_)) => {
return Ok(None);
}
Err(e) => {
return Err(e);
}
Expand Down Expand Up @@ -931,7 +928,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

match self.candidate_from_obligation(stack) {
Ok(Some(c)) => self.evaluate_candidate(stack, &c),
Err(SelectionError::Ambiguous(_)) => Ok(EvaluatedToAmbig),
Ok(None) => Ok(EvaluatedToAmbig),
Err(Overflow(OverflowError::Canonical)) => Err(OverflowError::Canonical),
Err(ErrorReporting) => Err(OverflowError::ErrorReporting),
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/error-codes/E0282.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
fn main() {
let x = "hello".chars().rev().collect(); //~ ERROR E0282
let x = "hello".chars().rev().collect();
//~^ ERROR E0282
}
4 changes: 3 additions & 1 deletion src/test/ui/error-codes/E0401.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ fn foo<T>(x: T) {
W: Fn()>
(y: T) { //~ ERROR E0401
}
bfnr(x); //~ ERROR type annotations needed
bfnr(x);
//~^ ERROR type annotations needed
//~| ERROR type annotations needed
}


Expand Down
27 changes: 24 additions & 3 deletions src/test/ui/error-codes/E0401.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | (y: T) {
| ^ use of generic parameter from outer function

error[E0401]: can't use generic parameters from outer function
--> $DIR/E0401.rs:22:25
--> $DIR/E0401.rs:24:25
|
LL | impl<T> Iterator for A<T> {
| ---- `Self` type implicitly declared here, by this `impl`
Expand All @@ -43,7 +43,28 @@ help: consider specifying the generic arguments
LL | bfnr::<U, V, W>(x);
| +++++++++++

error: aborting due to 4 previous errors
error[E0283]: type annotations needed
--> $DIR/E0401.rs:11:5
|
LL | bfnr(x);
| ^^^^ cannot infer type of the type parameter `W` declared on the function `bfnr`
|
= note: multiple `impl`s satisfying `_: Fn<()>` found in the following crates: `alloc`, `core`:
- impl<A, F> Fn<A> for &F
where A: Tuple, F: Fn<A>, F: ?Sized;
- impl<Args, F, A> Fn<Args> for Box<F, A>
where Args: Tuple, F: Fn<Args>, A: Allocator, F: ?Sized;
note: required by a bound in `bfnr`
--> $DIR/E0401.rs:4:30
|
LL | fn bfnr<U, V: Baz<U>, W: Fn()>(y: T) {
| ^^^^ required by this bound in `bfnr`
help: consider specifying the type arguments in the function call
|
LL | bfnr::<U, V, W>(x);
| +++++++++++

error: aborting due to 5 previous errors

Some errors have detailed explanations: E0282, E0401.
Some errors have detailed explanations: E0282, E0283, E0401.
For more information about an error, try `rustc --explain E0282`.
9 changes: 6 additions & 3 deletions src/test/ui/impl-trait/cross-return-site-inference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,19 @@ fn baa(b: bool) -> impl std::fmt::Debug {

fn muh() -> Result<(), impl std::fmt::Debug> {
Err("whoops")?;
Ok(()) //~ ERROR type annotations needed
Ok(())
//~^ ERROR type annotations needed
}

fn muh2() -> Result<(), impl std::fmt::Debug> {
return Err(From::from("foo")); //~ ERROR type annotations needed
return Err(From::from("foo"));
//~^ ERROR type annotations needed
Ok(())
}

fn muh3() -> Result<(), impl std::fmt::Debug> {
Err(From::from("foo")) //~ ERROR type annotations needed
Err(From::from("foo"))
//~^ ERROR type annotations needed
}

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/impl-trait/cross-return-site-inference.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ LL | Ok::<(), E>(())
| +++++++++

error[E0282]: type annotations needed
--> $DIR/cross-return-site-inference.rs:37:12
--> $DIR/cross-return-site-inference.rs:38:12
|
LL | return Err(From::from("foo"));
| ^^^ cannot infer type of the type parameter `E` declared on the enum `Result`
Expand All @@ -21,7 +21,7 @@ LL | return Err::<(), E>(From::from("foo"));
| +++++++++

error[E0282]: type annotations needed
--> $DIR/cross-return-site-inference.rs:42:5
--> $DIR/cross-return-site-inference.rs:44:5
|
LL | Err(From::from("foo"))
| ^^^ cannot infer type of the type parameter `E` declared on the enum `Result`
Expand Down
3 changes: 2 additions & 1 deletion src/test/ui/inference/cannot-infer-async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ fn main() {
let fut = async {
make_unit()?;

Ok(()) //~ ERROR type annotations needed
Ok(())
//~^ ERROR type annotations needed
};
}
3 changes: 2 additions & 1 deletion src/test/ui/inference/cannot-infer-closure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
fn main() {
let x = |a: (), b: ()| {
Err(a)?;
Ok(b) //~ ERROR type annotations needed
Ok(b)
//~^ ERROR type annotations needed
};
}
Loading