Skip to content

Commit

Permalink
Auto merge of #43880 - arielb1:noninvasive-probe, r=nikomatsakis
Browse files Browse the repository at this point in the history 
Remove the trait selection impl in method::probe

This removes the hacky trait selection reimplementation in `method::probe`, which occasionally comes and causes problems.

There are 2 issues I've found with this approach:
1. The older implementation sometimes had a "guess" type from an impl, which allowed subtyping to work. This is why I needed to make a change in `libtest`: there's an `impl<A> Clone for fn(A)` and we're calling `<for<'a> fn(&'a T) as Clone>::clone`. The older implementation would do a subtyping between the impl type and the trait type, so it would do the check for `<fn(A) as Clone>::clone`, and confirmation would continue with the subtyping. The newer implementation directly passes `<for<'a> fn(&'a T) as Clone>::clone` to selection, which fails. I'm not sure how big of a problem that would be in reality, especially after #43690 would remove the `Clone` problem, but I still want a crater run to avoid breaking the world.
2. The older implementation "looked into" impls to display error messages. I'm not sure that's an advantage - it looked exactly 1 level deep.

r? @eddyb
  • Loading branch information
@bors
bors committed Aug 30, 2017
2 parents c2f9cc4 + 15f6540 commit b58e31a
Show file tree
Hide file tree
Showing 12 changed files with 463 additions and 693 deletions.
186 changes: 80 additions & 106 deletions src/librustc/traits/select.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,12 @@ enum SelectionCandidate<'tcx> {
ProjectionCandidate,

/// Implementation of a `Fn`-family trait by one of the anonymous types
/// generated for a `||` expression. The ty::ClosureKind informs the
/// confirmation step what ClosureKind obligation to emit.
ClosureCandidate(/* closure */ DefId, ty::ClosureSubsts<'tcx>, ty::ClosureKind),
/// generated for a `||` expression.
ClosureCandidate,

/// Implementation of a `Generator` trait by one of the anonymous types
/// generated for a generator.
GeneratorCandidate(/* function / closure */ DefId, ty::ClosureSubsts<'tcx>),
GeneratorCandidate,

/// Implementation of a `Fn`-family trait by one of the anonymous
/// types generated for a fn pointer type (e.g., `fn(int)->int`)
Expand Down Expand Up @@ -229,20 +228,12 @@ impl<'a, 'tcx> ty::Lift<'tcx> for SelectionCandidate<'a> {
ObjectCandidate => ObjectCandidate,
BuiltinObjectCandidate => BuiltinObjectCandidate,
BuiltinUnsizeCandidate => BuiltinUnsizeCandidate,
ClosureCandidate => ClosureCandidate,
GeneratorCandidate => GeneratorCandidate,

ParamCandidate(ref trait_ref) => {
return tcx.lift(trait_ref).map(ParamCandidate);
}
GeneratorCandidate(def_id, ref substs) => {
return tcx.lift(substs).map(|substs| {
GeneratorCandidate(def_id, substs)
});
}
ClosureCandidate(def_id, ref substs, kind) => {
return tcx.lift(substs).map(|substs| {
ClosureCandidate(def_id, substs, kind)
});
}
})
}
}
Expand Down Expand Up @@ -1471,23 +1462,22 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// touch bound regions, they just capture the in-scope
// type/region parameters
let self_ty = *obligation.self_ty().skip_binder();
let (closure_def_id, substs) = match self_ty.sty {
ty::TyGenerator(id, substs, _) => (id, substs),
match self_ty.sty {
ty::TyGenerator(..) => {
debug!("assemble_generator_candidates: self_ty={:?} obligation={:?}",
self_ty,
obligation);

candidates.vec.push(GeneratorCandidate);
Ok(())
}
ty::TyInfer(ty::TyVar(_)) => {
debug!("assemble_generator_candidates: ambiguous self-type");
candidates.ambiguous = true;
return Ok(());
}
_ => { return Ok(()); }
};

debug!("assemble_generator_candidates: self_ty={:?} obligation={:?}",
self_ty,
obligation);

candidates.vec.push(GeneratorCandidate(closure_def_id, substs));

Ok(())
}
}

/// Check for the artificial impl that the compiler will create for an obligation like `X :
Expand All @@ -1509,36 +1499,31 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
// ok to skip binder because the substs on closure types never
// touch bound regions, they just capture the in-scope
// type/region parameters
let self_ty = *obligation.self_ty().skip_binder();
let (closure_def_id, substs) = match self_ty.sty {
ty::TyClosure(id, substs) => (id, substs),
match obligation.self_ty().skip_binder().sty {
ty::TyClosure(closure_def_id, _) => {
debug!("assemble_unboxed_candidates: kind={:?} obligation={:?}",
kind, obligation);
match self.infcx.closure_kind(closure_def_id) {
Some(closure_kind) => {
debug!("assemble_unboxed_candidates: closure_kind = {:?}", closure_kind);
if closure_kind.extends(kind) {
candidates.vec.push(ClosureCandidate);
}
}
None => {
debug!("assemble_unboxed_candidates: closure_kind not yet known");
candidates.vec.push(ClosureCandidate);
}
};
Ok(())
}
ty::TyInfer(ty::TyVar(_)) => {
debug!("assemble_unboxed_closure_candidates: ambiguous self-type");
candidates.ambiguous = true;
return Ok(());
}
_ => { return Ok(()); }
};

debug!("assemble_unboxed_candidates: self_ty={:?} kind={:?} obligation={:?}",
self_ty,
kind,
obligation);

match self.infcx.closure_kind(closure_def_id) {
Some(closure_kind) => {
debug!("assemble_unboxed_candidates: closure_kind = {:?}", closure_kind);
if closure_kind.extends(kind) {
candidates.vec.push(ClosureCandidate(closure_def_id, substs, kind));
}
}
None => {
debug!("assemble_unboxed_candidates: closure_kind not yet known");
candidates.vec.push(ClosureCandidate(closure_def_id, substs, kind));
}
}

Ok(())
}

/// Implement one of the `Fn()` family for a fn pointer.
Expand Down Expand Up @@ -1855,8 +1840,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
when there are other valid candidates");
}
ImplCandidate(..) |
ClosureCandidate(..) |
GeneratorCandidate(..) |
ClosureCandidate |
GeneratorCandidate |
FnPointerCandidate |
BuiltinObjectCandidate |
BuiltinUnsizeCandidate |
Expand Down Expand Up @@ -2198,15 +2183,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
Ok(VtableImpl(self.confirm_impl_candidate(obligation, impl_def_id)))
}

ClosureCandidate(closure_def_id, substs, kind) => {
let vtable_closure =
self.confirm_closure_candidate(obligation, closure_def_id, substs, kind)?;
ClosureCandidate => {
let vtable_closure = self.confirm_closure_candidate(obligation)?;
Ok(VtableClosure(vtable_closure))
}

GeneratorCandidate(closure_def_id, substs) => {
let vtable_generator =
self.confirm_generator_candidate(obligation, closure_def_id, substs)?;
GeneratorCandidate => {
let vtable_generator = self.confirm_generator_candidate(obligation)?;
Ok(VtableGenerator(vtable_generator))
}

Expand Down Expand Up @@ -2543,21 +2526,34 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}

fn confirm_generator_candidate(&mut self,
obligation: &TraitObligation<'tcx>,
closure_def_id: DefId,
substs: ty::ClosureSubsts<'tcx>)
-> Result<VtableGeneratorData<'tcx, PredicateObligation<'tcx>>,
obligation: &TraitObligation<'tcx>)
-> Result<VtableGeneratorData<'tcx, PredicateObligation<'tcx>>,
SelectionError<'tcx>>
{
// ok to skip binder because the substs on generator types never
// touch bound regions, they just capture the in-scope
// type/region parameters
let self_ty = self.infcx.shallow_resolve(obligation.self_ty().skip_binder());
let (closure_def_id, substs) = match self_ty.sty {
ty::TyGenerator(id, substs, _) => (id, substs),
_ => bug!("closure candidate for non-closure {:?}", obligation)
};

debug!("confirm_generator_candidate({:?},{:?},{:?})",
obligation,
closure_def_id,
substs);

let trait_ref =
self.generator_trait_ref_unnormalized(obligation, closure_def_id, substs);
let Normalized {
value: trait_ref,
obligations
} = self.generator_trait_ref(obligation, closure_def_id, substs);
} = normalize_with_depth(self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth+1,
&trait_ref);

debug!("confirm_generator_candidate(closure_def_id={:?}, trait_ref={:?}, obligations={:?})",
closure_def_id,
Expand All @@ -2577,22 +2573,36 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}

fn confirm_closure_candidate(&mut self,
obligation: &TraitObligation<'tcx>,
closure_def_id: DefId,
substs: ty::ClosureSubsts<'tcx>,
kind: ty::ClosureKind)
obligation: &TraitObligation<'tcx>)
-> Result<VtableClosureData<'tcx, PredicateObligation<'tcx>>,
SelectionError<'tcx>>
{
debug!("confirm_closure_candidate({:?},{:?},{:?})",
obligation,
closure_def_id,
substs);
debug!("confirm_closure_candidate({:?})", obligation);

let kind = match self.tcx().lang_items.fn_trait_kind(obligation.predicate.0.def_id()) {
Some(k) => k,
None => bug!("closure candidate for non-fn trait {:?}", obligation)
};

// ok to skip binder because the substs on closure types never
// touch bound regions, they just capture the in-scope
// type/region parameters
let self_ty = self.infcx.shallow_resolve(obligation.self_ty().skip_binder());
let (closure_def_id, substs) = match self_ty.sty {
ty::TyClosure(id, substs) => (id, substs),
_ => bug!("closure candidate for non-closure {:?}", obligation)
};

let trait_ref =
self.closure_trait_ref_unnormalized(obligation, closure_def_id, substs);
let Normalized {
value: trait_ref,
mut obligations
} = self.closure_trait_ref(obligation, closure_def_id, substs);
} = normalize_with_depth(self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth+1,
&trait_ref);

debug!("confirm_closure_candidate(closure_def_id={:?}, trait_ref={:?}, obligations={:?})",
closure_def_id,
Expand Down Expand Up @@ -3059,24 +3069,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ty::Binder(trait_ref)
}

fn closure_trait_ref(&mut self,
obligation: &TraitObligation<'tcx>,
closure_def_id: DefId,
substs: ty::ClosureSubsts<'tcx>)
-> Normalized<'tcx, ty::PolyTraitRef<'tcx>>
{
let trait_ref = self.closure_trait_ref_unnormalized(
obligation, closure_def_id, substs);

// A closure signature can contain associated types which
// must be normalized.
normalize_with_depth(self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth+1,
&trait_ref)
}

fn generator_trait_ref_unnormalized(&mut self,
obligation: &TraitObligation<'tcx>,
closure_def_id: DefId,
Expand All @@ -3098,24 +3090,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
ty::Binder(trait_ref)
}

fn generator_trait_ref(&mut self,
obligation: &TraitObligation<'tcx>,
closure_def_id: DefId,
substs: ty::ClosureSubsts<'tcx>)
-> Normalized<'tcx, ty::PolyTraitRef<'tcx>>
{
let trait_ref = self.generator_trait_ref_unnormalized(
obligation, closure_def_id, substs);

// A generator signature can contain associated types which
// must be normalized.
normalize_with_depth(self,
obligation.param_env,
obligation.cause.clone(),
obligation.recursion_depth+1,
&trait_ref)
}

/// Returns the obligations that are implied by instantiating an
/// impl or trait. The obligations are substituted and fully
/// normalized. This is used when confirming an impl or default
Expand Down
18 changes: 0 additions & 18 deletions src/librustc_typeck/check/method/confirm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,24 +232,6 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
})
}

probe::ExtensionImplPick(impl_def_id) => {
// The method being invoked is the method as defined on the trait,
// so return the substitutions from the trait. Consider:
//
// impl<A,B,C> Trait<A,B> for Foo<C> { ... }
//
// If we instantiate A, B, and C with $A, $B, and $C
// respectively, then we want to return the type
// parameters from the trait ([$A,$B]), not those from
// the impl ([$A,$B,$C]) not the receiver type ([$C]).
let impl_polytype = self.impl_self_ty(self.span, impl_def_id);
let impl_trait_ref =
self.instantiate_type_scheme(self.span,
impl_polytype.substs,
&self.tcx.impl_trait_ref(impl_def_id).unwrap());
impl_trait_ref.substs
}

probe::TraitPick => {
let trait_def_id = pick.item.container.id();

Expand Down
12 changes: 7 additions & 5 deletions src/librustc_typeck/check/method/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,16 @@ pub enum MethodError<'tcx> {
// Multiple methods might apply.
Ambiguity(Vec<CandidateSource>),

// Using a `Fn`/`FnMut`/etc method on a raw closure type before we have inferred its kind.
ClosureAmbiguity(// DefId of fn trait
DefId),

// Found an applicable method, but it is not visible. The second argument contains a list of
// not-in-scope traits which may work.
PrivateMatch(Def, Vec<DefId>),

// Found a `Self: Sized` bound where `Self` is a trait object, also the caller may have
// forgotten to import a trait.
IllegalSizedBound(Vec<DefId>),

// Found a match, but the return type is wrong
BadReturnType,
}

// Contains a list of static methods that may apply, a list of unsatisfied trait predicates which
Expand Down Expand Up @@ -113,9 +112,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
Ok(..) => true,
Err(NoMatch(..)) => false,
Err(Ambiguity(..)) => true,
Err(ClosureAmbiguity(..)) => true,
Err(PrivateMatch(..)) => allow_private,
Err(IllegalSizedBound(..)) => true,
Err(BadReturnType) => {
bug!("no return type expectations but got BadReturnType")
}

}
}

Expand Down
Loading

0 comments on commit b58e31a

Please sign in to comment.